lkml.org 
[lkml]   [2022]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 271/287] nbd: fix race between nbd_alloc_config() and module removal
    Date
    From: Yu Kuai <yukuai3@huawei.com>

    [ Upstream commit c55b2b983b0fa012942c3eb16384b2b722caa810 ]

    When nbd module is being removing, nbd_alloc_config() may be
    called concurrently by nbd_genl_connect(), although try_module_get()
    will return false, but nbd_alloc_config() doesn't handle it.

    The race may lead to the leak of nbd_config and its related
    resources (e.g, recv_workq) and oops in nbd_read_stat() due
    to the unload of nbd module as shown below:

    BUG: kernel NULL pointer dereference, address: 0000000000000040
    Oops: 0000 [#1] SMP PTI
    CPU: 5 PID: 13840 Comm: kworker/u17:33 Not tainted 5.14.0+ #1
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
    Workqueue: knbd16-recv recv_work [nbd]
    RIP: 0010:nbd_read_stat.cold+0x130/0x1a4 [nbd]
    Call Trace:
    recv_work+0x3b/0xb0 [nbd]
    process_one_work+0x1ed/0x390
    worker_thread+0x4a/0x3d0
    kthread+0x12a/0x150
    ret_from_fork+0x22/0x30

    Fixing it by checking the return value of try_module_get()
    in nbd_alloc_config(). As nbd_alloc_config() may return ERR_PTR(-ENODEV),
    assign nbd->config only when nbd_alloc_config() succeeds to ensure
    the value of nbd->config is binary (valid or NULL).

    Also adding a debug message to check the reference counter
    of nbd_config during module removal.

    Signed-off-by: Hou Tao <houtao1@huawei.com>
    Signed-off-by: Yu Kuai <yukuai3@huawei.com>
    Reviewed-by: Josef Bacik <josef@toxicpanda.com>
    Link: https://lore.kernel.org/r/20220521073749.3146892-3-yukuai3@huawei.com
    Signed-off-by: Jens Axboe <axboe@kernel.dk>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/block/nbd.c | 28 +++++++++++++++++++---------
    1 file changed, 19 insertions(+), 9 deletions(-)

    diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
    index 29bed6397173..f48553979b85 100644
    --- a/drivers/block/nbd.c
    +++ b/drivers/block/nbd.c
    @@ -1395,15 +1395,20 @@ static struct nbd_config *nbd_alloc_config(void)
    {
    struct nbd_config *config;

    + if (!try_module_get(THIS_MODULE))
    + return ERR_PTR(-ENODEV);
    +
    config = kzalloc(sizeof(struct nbd_config), GFP_NOFS);
    - if (!config)
    - return NULL;
    + if (!config) {
    + module_put(THIS_MODULE);
    + return ERR_PTR(-ENOMEM);
    + }
    +
    atomic_set(&config->recv_threads, 0);
    init_waitqueue_head(&config->recv_wq);
    init_waitqueue_head(&config->conn_wait);
    config->blksize = NBD_DEF_BLKSIZE;
    atomic_set(&config->live_connections, 0);
    - try_module_get(THIS_MODULE);
    return config;
    }

    @@ -1430,12 +1435,13 @@ static int nbd_open(struct block_device *bdev, fmode_t mode)
    mutex_unlock(&nbd->config_lock);
    goto out;
    }
    - config = nbd->config = nbd_alloc_config();
    - if (!config) {
    - ret = -ENOMEM;
    + config = nbd_alloc_config();
    + if (IS_ERR(config)) {
    + ret = PTR_ERR(config);
    mutex_unlock(&nbd->config_lock);
    goto out;
    }
    + nbd->config = config;
    refcount_set(&nbd->config_refs, 1);
    refcount_inc(&nbd->refs);
    mutex_unlock(&nbd->config_lock);
    @@ -1820,13 +1826,14 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
    nbd_put(nbd);
    return -EINVAL;
    }
    - config = nbd->config = nbd_alloc_config();
    - if (!nbd->config) {
    + config = nbd_alloc_config();
    + if (IS_ERR(config)) {
    mutex_unlock(&nbd->config_lock);
    nbd_put(nbd);
    printk(KERN_ERR "nbd: couldn't allocate config\n");
    - return -ENOMEM;
    + return PTR_ERR(config);
    }
    + nbd->config = config;
    refcount_set(&nbd->config_refs, 1);
    set_bit(NBD_BOUND, &config->runtime_flags);

    @@ -2352,6 +2359,9 @@ static void __exit nbd_cleanup(void)
    while (!list_empty(&del_list)) {
    nbd = list_first_entry(&del_list, struct nbd_device, list);
    list_del_init(&nbd->list);
    + if (refcount_read(&nbd->config_refs))
    + printk(KERN_ERR "nbd: possibly leaking nbd_config (ref %d)\n",
    + refcount_read(&nbd->config_refs));
    if (refcount_read(&nbd->refs) != 1)
    printk(KERN_ERR "nbd: possibly leaking a device\n");
    nbd_put(nbd);
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-06-13 14:23    [W:2.178 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site