Skip to content

Commit a2db328

Browse files
YuKuai-huaweiaxboe
authored andcommitted
null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues'
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: 45919fb ("null_blk: Enable modifying 'submit_queues' after an instance has been configured") Reported-and-tested-by: Yi Zhang <[email protected]> Closes: https://lore.kernel.org/all/CAHj4cs9LgsHLnjg8z06LQ3Pr5cax-+Ps+xT7AP7TPnEjStuwZA@mail.gmail.com/ Signed-off-by: Yu Kuai <[email protected]> Reviewed-by: Zhu Yanjun <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent 4a482e6 commit a2db328

File tree

1 file changed

+26
-14
lines changed

1 file changed

+26
-14
lines changed

drivers/block/null_blk/main.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -413,13 +413,25 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
413413
static int nullb_apply_submit_queues(struct nullb_device *dev,
414414
unsigned int submit_queues)
415415
{
416-
return nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
416+
int ret;
417+
418+
mutex_lock(&lock);
419+
ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
420+
mutex_unlock(&lock);
421+
422+
return ret;
417423
}
418424

419425
static int nullb_apply_poll_queues(struct nullb_device *dev,
420426
unsigned int poll_queues)
421427
{
422-
return nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
428+
int ret;
429+
430+
mutex_lock(&lock);
431+
ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
432+
mutex_unlock(&lock);
433+
434+
return ret;
423435
}
424436

425437
NULLB_DEVICE_ATTR(size, ulong, NULL);
@@ -468,28 +480,31 @@ static ssize_t nullb_device_power_store(struct config_item *item,
468480
if (ret < 0)
469481
return ret;
470482

483+
ret = count;
484+
mutex_lock(&lock);
471485
if (!dev->power && newp) {
472486
if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))
473-
return count;
487+
goto out;
488+
474489
ret = null_add_dev(dev);
475490
if (ret) {
476491
clear_bit(NULLB_DEV_FL_UP, &dev->flags);
477-
return ret;
492+
goto out;
478493
}
479494

480495
set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
481496
dev->power = newp;
482497
} else if (dev->power && !newp) {
483498
if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
484-
mutex_lock(&lock);
485499
dev->power = newp;
486500
null_del_dev(dev->nullb);
487-
mutex_unlock(&lock);
488501
}
489502
clear_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
490503
}
491504

492-
return count;
505+
out:
506+
mutex_unlock(&lock);
507+
return ret;
493508
}
494509

495510
CONFIGFS_ATTR(nullb_device_, power);
@@ -1932,15 +1947,12 @@ static int null_add_dev(struct nullb_device *dev)
19321947
nullb->q->queuedata = nullb;
19331948
blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q);
19341949

1935-
mutex_lock(&lock);
19361950
rv = ida_alloc(&nullb_indexes, GFP_KERNEL);
1937-
if (rv < 0) {
1938-
mutex_unlock(&lock);
1951+
if (rv < 0)
19391952
goto out_cleanup_disk;
1940-
}
1953+
19411954
nullb->index = rv;
19421955
dev->index = rv;
1943-
mutex_unlock(&lock);
19441956

19451957
if (config_item_name(&dev->group.cg_item)) {
19461958
/* Use configfs dir name as the device name */
@@ -1969,9 +1981,7 @@ static int null_add_dev(struct nullb_device *dev)
19691981
if (rv)
19701982
goto out_ida_free;
19711983

1972-
mutex_lock(&lock);
19731984
list_add_tail(&nullb->list, &nullb_list);
1974-
mutex_unlock(&lock);
19751985

19761986
pr_info("disk %s created\n", nullb->disk_name);
19771987

@@ -2020,7 +2030,9 @@ static int null_create_dev(void)
20202030
if (!dev)
20212031
return -ENOMEM;
20222032

2033+
mutex_lock(&lock);
20232034
ret = null_add_dev(dev);
2035+
mutex_unlock(&lock);
20242036
if (ret) {
20252037
null_free_dev(dev);
20262038
return ret;

0 commit comments

Comments
 (0)