Re: [PATCH V4 8/9] null_blk: serialize configfs attribute stores with device setup

[Nilay Shroff]

On 7/9/26 3:34 PM, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@xxxxxxxxxx>
>
> 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.
>
> Also reset ret to 0 after the input parsing so that within the locked
> section ret is purely a status code, rather than carrying the byte count
> returned by nullb_device_##TYPE##_attr_store(). The field is then written
> only on success via if (!ret), giving a single consistent rule for both the
> apply_fn and the APPLY=NULL paths.
>
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>

Looks good to me.

Reviewed-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>