lkml.org 
[lkml]   [2018]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 054/254] dm: fix various targets to dm_register_target after module __init resources created
    3.16.55-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: "monty_pavel@sina.com" <monty_pavel@sina.com>

    commit 7e6358d244e4706fe612a77b9c36519a33600ac0 upstream.

    A NULL pointer is seen if two concurrent "vgchange -ay -K <vg name>"
    processes race to load the dm-thin-pool module:

    PID: 25992 TASK: ffff883cd7d23500 CPU: 4 COMMAND: "vgchange"
    #0 [ffff883cd743d600] machine_kexec at ffffffff81038fa9
    0000001 [ffff883cd743d660] crash_kexec at ffffffff810c5992
    0000002 [ffff883cd743d730] oops_end at ffffffff81515c90
    0000003 [ffff883cd743d760] no_context at ffffffff81049f1b
    0000004 [ffff883cd743d7b0] __bad_area_nosemaphore at ffffffff8104a1a5
    0000005 [ffff883cd743d800] bad_area at ffffffff8104a2ce
    0000006 [ffff883cd743d830] __do_page_fault at ffffffff8104aa6f
    0000007 [ffff883cd743d950] do_page_fault at ffffffff81517bae
    0000008 [ffff883cd743d980] page_fault at ffffffff81514f95
    [exception RIP: kmem_cache_alloc+108]
    RIP: ffffffff8116ef3c RSP: ffff883cd743da38 RFLAGS: 00010046
    RAX: 0000000000000004 RBX: ffffffff81121b90 RCX: ffff881bf1e78cc0
    RDX: 0000000000000000 RSI: 00000000000000d0 RDI: 0000000000000000
    RBP: ffff883cd743da68 R8: ffff881bf1a4eb00 R9: 0000000080042000
    R10: 0000000000002000 R11: 0000000000000000 R12: 00000000000000d0
    R13: 0000000000000000 R14: 00000000000000d0 R15: 0000000000000246
    ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
    0000009 [ffff883cd743da70] mempool_alloc_slab at ffffffff81121ba5
    0000010 [ffff883cd743da80] mempool_create_node at ffffffff81122083
    0000011 [ffff883cd743dad0] mempool_create at ffffffff811220f4
    0000012 [ffff883cd743dae0] pool_ctr at ffffffffa08de049 [dm_thin_pool]
    0000013 [ffff883cd743dbd0] dm_table_add_target at ffffffffa0005f2f [dm_mod]
    0000014 [ffff883cd743dc30] table_load at ffffffffa0008ba9 [dm_mod]
    0000015 [ffff883cd743dc90] ctl_ioctl at ffffffffa0009dc4 [dm_mod]

    The race results in a NULL pointer because:

    Process A (vgchange -ay -K):
    a. send DM_LIST_VERSIONS_CMD ioctl;
    b. pool_target not registered;
    c. modprobe dm_thin_pool and wait until end.

    Process B (vgchange -ay -K):
    a. send DM_LIST_VERSIONS_CMD ioctl;
    b. pool_target registered;
    c. table_load->dm_table_add_target->pool_ctr;
    d. _new_mapping_cache is NULL and panic.
    Note:
    1. process A and process B are two concurrent processes.
    2. pool_target can be detected by process B but
    _new_mapping_cache initialization has not ended.

    To fix dm-thin-pool, and other targets (cache, multipath, and snapshot)
    with the same problem, simply dm_register_target() after all resources
    created during module init (as labelled with __init) are finished.

    Signed-off-by: monty <monty_pavel@sina.com>
    Signed-off-by: Mike Snitzer <snitzer@redhat.com>
    [bwh: Backported to 3.16: adjust context]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    drivers/md/dm-cache-target.c | 12 +++++------
    drivers/md/dm-mpath.c | 18 ++++++++---------
    drivers/md/dm-snap.c | 48 ++++++++++++++++++++++----------------------
    drivers/md/dm-thin.c | 22 +++++++++-----------
    4 files changed, 49 insertions(+), 51 deletions(-)

    --- a/drivers/md/dm-cache-target.c
    +++ b/drivers/md/dm-cache-target.c
    @@ -3108,18 +3108,18 @@ static int __init dm_cache_init(void)
    {
    int r;

    - r = dm_register_target(&cache_target);
    - if (r) {
    - DMERR("cache target registration failed: %d", r);
    - return r;
    - }
    -
    migration_cache = KMEM_CACHE(dm_cache_migration, 0);
    if (!migration_cache) {
    dm_unregister_target(&cache_target);
    return -ENOMEM;
    }

    + r = dm_register_target(&cache_target);
    + if (r) {
    + DMERR("cache target registration failed: %d", r);
    + return r;
    + }
    +
    return 0;
    }

    --- a/drivers/md/dm-mpath.c
    +++ b/drivers/md/dm-mpath.c
    @@ -1688,13 +1688,6 @@ static int __init dm_multipath_init(void
    if (!_mpio_cache)
    return -ENOMEM;

    - r = dm_register_target(&multipath_target);
    - if (r < 0) {
    - DMERR("register failed %d", r);
    - r = -EINVAL;
    - goto bad_register_target;
    - }
    -
    kmultipathd = alloc_workqueue("kmpathd", WQ_MEM_RECLAIM, 0);
    if (!kmultipathd) {
    DMERR("failed to create workqueue kmpathd");
    @@ -1716,17 +1709,24 @@ static int __init dm_multipath_init(void
    goto bad_alloc_kmpath_handlerd;
    }

    + r = dm_register_target(&multipath_target);
    + if (r < 0) {
    + DMERR("register failed %d", r);
    + r = -EINVAL;
    + goto bad_register_target;
    + }
    +
    DMINFO("version %u.%u.%u loaded",
    multipath_target.version[0], multipath_target.version[1],
    multipath_target.version[2]);

    return 0;

    +bad_register_target:
    + destroy_workqueue(kmpath_handlerd);
    bad_alloc_kmpath_handlerd:
    destroy_workqueue(kmultipathd);
    bad_alloc_kmultipathd:
    - dm_unregister_target(&multipath_target);
    -bad_register_target:
    kmem_cache_destroy(_mpio_cache);

    return r;
    --- a/drivers/md/dm-snap.c
    +++ b/drivers/md/dm-snap.c
    @@ -2404,24 +2404,6 @@ static int __init dm_snapshot_init(void)
    return r;
    }

    - r = dm_register_target(&snapshot_target);
    - if (r < 0) {
    - DMERR("snapshot target register failed %d", r);
    - goto bad_register_snapshot_target;
    - }
    -
    - r = dm_register_target(&origin_target);
    - if (r < 0) {
    - DMERR("Origin target register failed %d", r);
    - goto bad_register_origin_target;
    - }
    -
    - r = dm_register_target(&merge_target);
    - if (r < 0) {
    - DMERR("Merge target register failed %d", r);
    - goto bad_register_merge_target;
    - }
    -
    r = init_origin_hash();
    if (r) {
    DMERR("init_origin_hash failed.");
    @@ -2442,19 +2424,37 @@ static int __init dm_snapshot_init(void)
    goto bad_pending_cache;
    }

    + r = dm_register_target(&snapshot_target);
    + if (r < 0) {
    + DMERR("snapshot target register failed %d", r);
    + goto bad_register_snapshot_target;
    + }
    +
    + r = dm_register_target(&origin_target);
    + if (r < 0) {
    + DMERR("Origin target register failed %d", r);
    + goto bad_register_origin_target;
    + }
    +
    + r = dm_register_target(&merge_target);
    + if (r < 0) {
    + DMERR("Merge target register failed %d", r);
    + goto bad_register_merge_target;
    + }
    +
    return 0;

    -bad_pending_cache:
    - kmem_cache_destroy(exception_cache);
    -bad_exception_cache:
    - exit_origin_hash();
    -bad_origin_hash:
    - dm_unregister_target(&merge_target);
    bad_register_merge_target:
    dm_unregister_target(&origin_target);
    bad_register_origin_target:
    dm_unregister_target(&snapshot_target);
    bad_register_snapshot_target:
    + kmem_cache_destroy(pending_cache);
    +bad_pending_cache:
    + kmem_cache_destroy(exception_cache);
    +bad_exception_cache:
    + exit_origin_hash();
    +bad_origin_hash:
    dm_exception_store_exit();

    return r;
    --- a/drivers/md/dm-thin.c
    +++ b/drivers/md/dm-thin.c
    @@ -3521,30 +3521,28 @@ static struct target_type thin_target =

    static int __init dm_thin_init(void)
    {
    - int r;
    + int r = -ENOMEM;

    pool_table_init();

    + _new_mapping_cache = KMEM_CACHE(dm_thin_new_mapping, 0);
    + if (!_new_mapping_cache)
    + return r;
    +
    r = dm_register_target(&thin_target);
    if (r)
    - return r;
    + goto bad_new_mapping_cache;

    r = dm_register_target(&pool_target);
    if (r)
    - goto bad_pool_target;
    -
    - r = -ENOMEM;
    -
    - _new_mapping_cache = KMEM_CACHE(dm_thin_new_mapping, 0);
    - if (!_new_mapping_cache)
    - goto bad_new_mapping_cache;
    + goto bad_thin_target;

    return 0;

    -bad_new_mapping_cache:
    - dm_unregister_target(&pool_target);
    -bad_pool_target:
    +bad_thin_target:
    dm_unregister_target(&thin_target);
    +bad_new_mapping_cache:
    + kmem_cache_destroy(_new_mapping_cache);

    return r;
    }
    \
     
     \ /
      Last update: 2018-02-28 17:51    [W:4.102 / U:1.404 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site