|
| 1 | +null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues' |
| 2 | + |
| 3 | +jira LE-1907 |
| 4 | +Rebuild_History Non-Buildable kernel-5.14.0-427.40.1.el9_4 |
| 5 | +commit-author Yu Kuai <yukuai3@huawei.com> |
| 6 | +commit a2db328b0839312c169eb42746ec46fc1ab53ed2 |
| 7 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 8 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 9 | +ciq/ciq_backports/kernel-5.14.0-427.40.1.el9_4/a2db328b.failed |
| 10 | + |
| 11 | +Writing 'power' and 'submit_queues' concurrently will trigger kernel |
| 12 | +panic: |
| 13 | + |
| 14 | +Test script: |
| 15 | + |
| 16 | +modprobe null_blk nr_devices=0 |
| 17 | +mkdir -p /sys/kernel/config/nullb/nullb0 |
| 18 | +while true; do echo 1 > submit_queues; echo 4 > submit_queues; done & |
| 19 | +while true; do echo 1 > power; echo 0 > power; done |
| 20 | + |
| 21 | +Test result: |
| 22 | + |
| 23 | +BUG: kernel NULL pointer dereference, address: 0000000000000148 |
| 24 | +Oops: 0000 [#1] PREEMPT SMP |
| 25 | +RIP: 0010:__lock_acquire+0x41d/0x28f0 |
| 26 | +Call Trace: |
| 27 | + <TASK> |
| 28 | + lock_acquire+0x121/0x450 |
| 29 | + down_write+0x5f/0x1d0 |
| 30 | + simple_recursive_removal+0x12f/0x5c0 |
| 31 | + blk_mq_debugfs_unregister_hctxs+0x7c/0x100 |
| 32 | + blk_mq_update_nr_hw_queues+0x4a3/0x720 |
| 33 | + nullb_update_nr_hw_queues+0x71/0xf0 [null_blk] |
| 34 | + nullb_device_submit_queues_store+0x79/0xf0 [null_blk] |
| 35 | + configfs_write_iter+0x119/0x1e0 |
| 36 | + vfs_write+0x326/0x730 |
| 37 | + ksys_write+0x74/0x150 |
| 38 | + |
| 39 | +This is because del_gendisk() can concurrent with |
| 40 | +blk_mq_update_nr_hw_queues(): |
| 41 | + |
| 42 | +nullb_device_power_store nullb_apply_submit_queues |
| 43 | + null_del_dev |
| 44 | + del_gendisk |
| 45 | + nullb_update_nr_hw_queues |
| 46 | + if (!dev->nullb) |
| 47 | + // still set while gendisk is deleted |
| 48 | + return 0 |
| 49 | + blk_mq_update_nr_hw_queues |
| 50 | + dev->nullb = NULL |
| 51 | + |
| 52 | +Fix this problem by resuing the global mutex to protect |
| 53 | +nullb_device_power_store() and nullb_update_nr_hw_queues() from configfs. |
| 54 | + |
| 55 | +Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured") |
| 56 | +Reported-and-tested-by: Yi Zhang <yi.zhang@redhat.com> |
| 57 | +Closes: https://lore.kernel.org/all/CAHj4cs9LgsHLnjg8z06LQ3Pr5cax-+Ps+xT7AP7TPnEjStuwZA@mail.gmail.com/ |
| 58 | + Signed-off-by: Yu Kuai <yukuai3@huawei.com> |
| 59 | + Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> |
| 60 | +Link: https://lore.kernel.org/r/20240523153934.1937851-1-yukuai1@huaweicloud.com |
| 61 | + Signed-off-by: Jens Axboe <axboe@kernel.dk> |
| 62 | +(cherry picked from commit a2db328b0839312c169eb42746ec46fc1ab53ed2) |
| 63 | + Signed-off-by: Jonathan Maple <jmaple@ciq.com> |
| 64 | + |
| 65 | +# Conflicts: |
| 66 | +# drivers/block/null_blk/main.c |
| 67 | +diff --cc drivers/block/null_blk/main.c |
| 68 | +index 86b661d289b9,eb023d267369..000000000000 |
| 69 | +--- a/drivers/block/null_blk/main.c |
| 70 | ++++ b/drivers/block/null_blk/main.c |
| 71 | +@@@ -2146,28 -1947,13 +2161,34 @@@ static int null_add_dev(struct nullb_de |
| 72 | + nullb->q->queuedata = nullb; |
| 73 | + blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q); |
| 74 | + |
| 75 | +++<<<<<<< HEAD |
| 76 | + + mutex_lock(&lock); |
| 77 | + + rv = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL); |
| 78 | + + if (rv < 0) { |
| 79 | + + mutex_unlock(&lock); |
| 80 | + + goto out_cleanup_zone; |
| 81 | + + } |
| 82 | +++======= |
| 83 | ++ rv = ida_alloc(&nullb_indexes, GFP_KERNEL); |
| 84 | ++ if (rv < 0) |
| 85 | ++ goto out_cleanup_disk; |
| 86 | ++ |
| 87 | +++>>>>>>> a2db328b0839 (null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues') |
| 88 | + nullb->index = rv; |
| 89 | + dev->index = rv; |
| 90 | +- mutex_unlock(&lock); |
| 91 | + |
| 92 | + + blk_queue_logical_block_size(nullb->q, dev->blocksize); |
| 93 | + + blk_queue_physical_block_size(nullb->q, dev->blocksize); |
| 94 | + + if (!dev->max_sectors) |
| 95 | + + dev->max_sectors = queue_max_hw_sectors(nullb->q); |
| 96 | + + dev->max_sectors = min(dev->max_sectors, BLK_DEF_MAX_SECTORS); |
| 97 | + + blk_queue_max_hw_sectors(nullb->q, dev->max_sectors); |
| 98 | + + |
| 99 | + + if (dev->virt_boundary) |
| 100 | + + blk_queue_virt_boundary(nullb->q, PAGE_SIZE - 1); |
| 101 | + + |
| 102 | + + null_config_discard(nullb); |
| 103 | + + |
| 104 | + if (config_item_name(&dev->group.cg_item)) { |
| 105 | + /* Use configfs dir name as the device name */ |
| 106 | + snprintf(nullb->disk_name, sizeof(nullb->disk_name), |
| 107 | +* Unmerged path drivers/block/null_blk/main.c |
0 commit comments