diff options
| author | Zizhi Wo <wozizhi@huawei.com> | 2026-07-25 10:25:08 +0800 |
|---|---|---|
| committer | Jens Axboe <axboe@kernel.dk> | 2026-08-15 17:06:51 -0600 |
| commit | 7e7fff51808237703a3a1df6dd5cae1dfd1db86d (patch) | |
| tree | e2bb2bebfc6bfb73bdef6e9a45f38be7e5c66e62 | |
| parent | e3ef4b1b76d6721067259577ab5df54a173d9341 (diff) | |
| download | linux-7e7fff51808237703a3a1df6dd5cae1dfd1db86d.tar.gz linux-7e7fff51808237703a3a1df6dd5cae1dfd1db86d.zip | |
null_blk: serialize configfs attribute stores with the lock
The NULLB_DEVICE_ATTR _store takes no lock: apply_fn attributes
(submit_queues, poll_queues) get dev->NAME written again after apply_fn
returns, outside its lock; APPLY=NULL attributes are entirely lockless.
configfs only serializes stores per-open-file, so concurrent stores on
separate fds race.
For apply_fn attributes, once one store's apply_fn has reconfigured the
hardware, a second (losing) store can still overwrite dev->NAME
afterwards. This leaves dev->submit_queues out of sync with the live
queue count, which is later caught by the WARN_ON_ONCE() in
null_map_queues().
For !apply_fn attributes, power_store()'s null_add_dev() validates and
builds the device under "lock" but only sets CONFIGURED afterwards. A store
slipping in during this window can change a field mid-setup -- for example,
zone_nr_conv can be pushed above nr_zones after it has already been
clamped, leading to an out-of-bounds dev->zones[] access.
Take "lock" in the macro around the apply_fn call, the CONFIGURED test and
the field write, and move it out of nullb_apply_submit_queues()/
nullb_apply_poll_queues() so both paths are covered once. This serializes
stores with power_store's setup and with each other.
Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-10-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
| -rw-r--r-- | drivers/block/null_blk/main.c | 17 |
1 files changed, 3 insertions, 14 deletions
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 962b0e05ce50..c7dfbec3e7d6 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -360,6 +360,7 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \ ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\ if (ret < 0) \ return ret; \ + guard(mutex)(&lock); \ if (apply_fn) \ ret = apply_fn(dev, new_value); \ else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \ @@ -430,25 +431,13 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev, static int nullb_apply_submit_queues(struct nullb_device *dev, unsigned int submit_queues) { - int ret; - - mutex_lock(&lock); - ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues); - mutex_unlock(&lock); - - return ret; + return nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues); } static int nullb_apply_poll_queues(struct nullb_device *dev, unsigned int poll_queues) { - int ret; - - mutex_lock(&lock); - ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues); - mutex_unlock(&lock); - - return ret; + return nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues); } NULLB_DEVICE_ATTR(size, ulong, NULL); |
