Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8a7da4ce authored by Yu Kuai's avatar Yu Kuai Committed by Greg Kroah-Hartman
Browse files

nbd: fix race between nbd_alloc_config() and module removal 



[ 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: default avatarHou Tao <houtao1@huawei.com>
Signed-off-by: default avatarYu Kuai <yukuai3@huawei.com>
Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
Link: https://lore.kernel.org/r/20220521073749.3146892-3-yukuai3@huawei.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent 1be608e1

drivers/block/nbd.c

+19 −9
Original line number Diff line number Diff line
@@ -1457,15 +1457,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;

}
@@ -1492,12 +1497,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);
@@ -1919,13 +1925,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_RT_BOUND, &config->runtime_flags);
@@ -2461,6 +2468,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);