Commit 7de60ff5 authored by Yu Kuai's avatar Yu Kuai Committed by Li Nan
Browse files

null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues' 

mainline inclusion
from mainline-v6.10-rc1
commit a2db328b0839312c169eb42746ec46fc1ab53ed2
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IA7D6H
CVE: CVE-2024-36478

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a2db328b0839312c169eb42746ec46fc1ab53ed2



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

Writing 'power' and 'submit_queues' concurrently will trigger kernel
panic:

Test script:

modprobe null_blk nr_devices=0
mkdir -p /sys/kernel/config/nullb/nullb0
while true; do echo 1 > submit_queues; echo 4 > submit_queues; done &
while true; do echo 1 > power; echo 0 > power; done

Test result:

BUG: kernel NULL pointer dereference, address: 0000000000000148
Oops: 0000 [#1] PREEMPT SMP
RIP: 0010:__lock_acquire+0x41d/0x28f0
Call Trace:
 <TASK>
 lock_acquire+0x121/0x450
 down_write+0x5f/0x1d0
 simple_recursive_removal+0x12f/0x5c0
 blk_mq_debugfs_unregister_hctxs+0x7c/0x100
 blk_mq_update_nr_hw_queues+0x4a3/0x720
 nullb_update_nr_hw_queues+0x71/0xf0 [null_blk]
 nullb_device_submit_queues_store+0x79/0xf0 [null_blk]
 configfs_write_iter+0x119/0x1e0
 vfs_write+0x326/0x730
 ksys_write+0x74/0x150

This is because del_gendisk() can concurrent with
blk_mq_update_nr_hw_queues():

nullb_device_power_store	nullb_apply_submit_queues
 null_del_dev
 del_gendisk
				 nullb_update_nr_hw_queues
				  if (!dev->nullb)
				  // still set while gendisk is deleted
				   return 0
				  blk_mq_update_nr_hw_queues
 dev->nullb = NULL

Fix this problem by resuing the global mutex to protect
nullb_device_power_store() and nullb_update_nr_hw_queues() from configfs.

Fixes: 45919fbf ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Reported-and-tested-by: default avatarYi Zhang <yi.zhang@redhat.com>
Closes: https://lore.kernel.org/all/CAHj4cs9LgsHLnjg8z06LQ3Pr5cax-+Ps+xT7AP7TPnEjStuwZA@mail.gmail.com/


Signed-off-by: default avatarYu Kuai <yukuai3@huawei.com>
Reviewed-by: default avatarZhu Yanjun <yanjun.zhu@linux.dev>
Link: https://lore.kernel.org/r/20240523153934.1937851-1-yukuai1@huaweicloud.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Conflicts:
	drivers/block/null_blk/main.c
[ A lot of conflict, this patch just expand the protection range of
mutex, ignore conflict. ]
Signed-off-by: default avatarLi Nan <linan122@huawei.com>
parent 1cc61a82

drivers/block/null_blk/main.c

+26 −17
Original line number Diff line number Diff line
@@ -323,11 +323,9 @@ CONFIGFS_ATTR(nullb_device_, NAME); 
static int nullb_apply_submit_queues(struct nullb_device *dev,

				     unsigned int submit_queues)

{

	struct nullb *nullb = dev->nullb;

	struct nullb *nullb;

	struct blk_mq_tag_set *set;



	if (!nullb)

		return 0;

	int ret = 0;



	/*

	 * Make sure that null_init_hctx() does not access nullb->queues[] past
@@ -335,9 +333,20 @@ static int nullb_apply_submit_queues(struct nullb_device *dev, 
	 */

	if (submit_queues > nr_cpu_ids)

		return -EINVAL;



	mutex_lock(&lock);



	nullb = dev->nullb;

	if (!nullb)

		goto out;



	set = nullb->tag_set;

	blk_mq_update_nr_hw_queues(set, submit_queues);

	return set->nr_hw_queues == submit_queues ? 0 : -ENOMEM;

	ret = set->nr_hw_queues == submit_queues ? 0 : -ENOMEM;



out:

	mutex_unlock(&lock);

	return ret;

}



NULLB_DEVICE_ATTR(size, ulong, NULL);
@@ -378,27 +387,31 @@ static ssize_t nullb_device_power_store(struct config_item *item, 
	if (ret < 0)

		return ret;



	ret = count;

	mutex_lock(&lock);

	if (!dev->power && newp) {

		if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))

			return count;

			goto out;



		if (null_add_dev(dev)) {

			clear_bit(NULLB_DEV_FL_UP, &dev->flags);

			return -ENOMEM;

			ret = -ENOMEM;

			goto out;

		}



		set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);

		dev->power = newp;

	} else if (dev->power && !newp) {

		if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {

			mutex_lock(&lock);

			dev->power = newp;

			null_del_dev(dev->nullb);

			mutex_unlock(&lock);

		}

		clear_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);

	}



	return count;

out:

	mutex_unlock(&lock);

	return ret;

}



CONFIGFS_ATTR(nullb_device_, power);
@@ -1880,15 +1893,11 @@ static int null_add_dev(struct nullb_device *dev) 
	blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q);

	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, nullb->q);



	mutex_lock(&lock);

	rv = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL);

	if (rv < 0) {

		mutex_unlock(&lock);

	if (rv < 0)

		goto out_cleanup_zone;

	}

	nullb->index = rv;

	dev->index = rv;

	mutex_unlock(&lock);



	blk_queue_logical_block_size(nullb->q, dev->blocksize);

	blk_queue_physical_block_size(nullb->q, dev->blocksize);
@@ -1901,9 +1910,7 @@ static int null_add_dev(struct nullb_device *dev) 
	if (rv)

		goto out_ida_free;



	mutex_lock(&lock);

	list_add_tail(&nullb->list, &nullb_list);

	mutex_unlock(&lock);



	return 0;
@@ -1985,7 +1992,9 @@ static int __init null_init(void) 
			ret = -ENOMEM;

			goto err_dev;

		}

		mutex_lock(&lock);

		ret = null_add_dev(dev);

		mutex_unlock(&lock);

		if (ret) {

			null_free_dev(dev);

			goto err_dev;