From: Zizhi Wo 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 the loser can overwrite dev->NAME after the winner's apply_fn reconfigured hardware, mismatching dev->submit_queues with the live queue count (null_map_queues WARN_ON_ONCE). For APPLY=NULL attributes, a store during power_store's null_add_dev() (validates and builds the device under "lock" but sets CONFIGURED only afterwards) can change a field mid-setup -- e.g. zone_nr_conv pushed above nr_zones after clamping, causing 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. _show still takes no lock; its data races are handled by READ_ONCE/ WRITE_ONCE in a follow-up. Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured") Signed-off-by: Zizhi Wo --- drivers/block/null_blk/main.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index df85189f0b69..9e0002e4aeec 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -360,13 +360,16 @@ 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; \ + mutex_lock(&lock); \ if (apply_fn) \ ret = apply_fn(dev, new_value); \ else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \ ret = -EBUSY; \ + if (!ret) \ + dev->NAME = new_value; \ + mutex_unlock(&lock); \ if (ret < 0) \ return ret; \ - dev->NAME = new_value; \ return count; \ } \ CONFIGFS_ATTR(nullb_device_, NAME); @@ -421,25 +424,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); -- 2.52.0