From: Zizhi Wo The _show callback in the NULLB_DEVICE_ATTR macro reads dev->NAME and the _store path writes it. configfs does not serialize accesses across separate open file descriptions (buffer->mutex is per-fd), and _show takes no lock, so a concurrent read and write on the same attribute is a data race. The _show readers also race against writes to these fields that run after the configfs item becomes visible, e.g. in nullb_update_nr_hw_queues(). All of those writers now run under the file-scope lock: _store takes it unconditionally, and the setup-side writers run under power_store() which holds the same lock. The only remaining unsynchronized accesses are the plain reads in _show. Rather than annotating every field with READ_ONCE()/WRITE_ONCE() across files, simply take the file-scope lock in _show (and in power_show) as well. This closes the remaining _show-vs-write data races with a single lock and keeps the writers as plain assignments. configfs attribute access is not on the I/O hot path, so taking the mutex in _show is acceptable from a performance standpoint. The dev fields written in null_alloc_dev() and dev->power in nullb_group_drop_item() need no locking: the former runs from .make_group before the item is published, and the latter is serialized by configfs frag_sem/frag_dead against attribute show/store. Suggested-by: Nilay Shroff Signed-off-by: Zizhi Wo Reviewed-by: Nilay Shroff --- drivers/block/null_blk/main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 67cd32d28887..c8487a630e1a 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -345,8 +345,14 @@ static ssize_t nullb_device_bool_attr_store(bool *val, const char *page, static ssize_t \ nullb_device_##NAME##_show(struct config_item *item, char *page) \ { \ - return nullb_device_##TYPE##_attr_show( \ + ssize_t ret; \ + \ + mutex_lock(&lock); \ + ret = nullb_device_##TYPE##_attr_show( \ to_nullb_device(item)->NAME, page); \ + mutex_unlock(&lock); \ + \ + return ret; \ } \ static ssize_t \ nullb_device_##NAME##_store(struct config_item *item, const char *page, \ @@ -479,7 +485,13 @@ NULLB_DEVICE_ATTR(badblocks_partial_io, bool, NULL); static ssize_t nullb_device_power_show(struct config_item *item, char *page) { - return nullb_device_bool_attr_show(to_nullb_device(item)->power, page); + ssize_t ret; + + mutex_lock(&lock); + ret = nullb_device_bool_attr_show(to_nullb_device(item)->power, page); + mutex_unlock(&lock); + + return ret; } static ssize_t nullb_device_power_store(struct config_item *item, -- 2.52.0