[PATCH V2 0/6] null_blk: fix init/exit races, leaks and cleanup

[PATCH V2 0/6] null_blk: fix init/exit races, leaks and cleanup

[Zizhi Wo]

This series fixes several issues in null_blk around lock initialization,
concurrent configfs access, and module init/exit cleanup.

Patches 1-2 fix the uninitialized mutex. Following Bart's suggestion, the
fix now uses DEFINE_MUTEX() and renames the lock.

Patch 3 fixes configfs registration concurrency.

Patch 4 reorders resource release in null_exit() to match null_init().

Patch 5 fixes a data race where the NULLB_DEVICE_ATTR macro locklessly
overwrote dev state already set under the lock by apply_fn().

Patch 6 fixes a global tag_set leak on the null_init() error path.

See the individual patch descriptions for details.

Changes since v1:
- Added patches 4-6, and modify the lock name in patch 2.
https://lore.kernel.org/all/20260706123507.3809871-1-wozizhi@huaweicloud.com/

Zizhi Wo (6):
  null_blk: use DEFINE_MUTEX for the file-scope mutex
  null_blk: give the file-scope mutex a descriptive name
  null_blk: register configfs subsystem after creating default devices
  null_blk: move unregister_blkdev() after destroying dev in null_exit()
  null_blk: don't locklessly overwrite dev state after apply_fn
  null_blk: free global tag_set on init error path

 drivers/block/null_blk/main.c | 70 +++++++++++++++++------------------
 1 file changed, 34 insertions(+), 36 deletions(-)

-- 
2.52.0

[PATCH V2 1/6] null_blk: use DEFINE_MUTEX for the file-scope mutex

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

In null_init(), mutex_init(&lock) currently happens after
configfs_register_subsystem(), which exposes the nullb subsystem to
userspace. A racing mkdir() into /sys/kernel/config/nullb/ can reach
null_find_dev_by_name() -> mutex_lock(&lock) before the mutex is
initialized, trigger warning:

[  123.137788] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
[  123.137796] WARNING: kernel/locking/mutex.c:159 at mutex_lock+0x171/0x1c0, CPU#13: mkdir/1301
[  123.140090] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4
......
[  123.154926] Call Trace:
[  123.155172]  <TASK>
[  123.155419]  ? __pfx_mutex_lock+0x10/0x10
[  123.156181]  ? __pfx__raw_spin_lock+0x10/0x10
[  123.156571]  nullb_group_make_group+0x20/0x100 [null_blk]
[  123.157011]  configfs_mkdir+0x47b/0xc70
[  123.157337]  ? __pfx_configfs_mkdir+0x10/0x10
[  123.157719]  ? may_create_dentry+0x242/0x2e0
[  123.158061]  vfs_mkdir+0x2a9/0x6c0
[  123.158352]  filename_mkdirat+0x3dc/0x500
[  123.158710]  ? __pfx_filename_mkdirat+0x10/0x10
[  123.159070]  ? strncpy_from_user+0x3a/0x1d0
[  123.159413]  __x64_sys_mkdir+0x6b/0x90
[  123.159760]  do_syscall_64+0xea/0x600

Replace the runtime mutex_init(&lock) with a static DEFINE_MUTEX(lock)
declaration to fix this issue.

Fixes: 49c3b9266a71 ("block: null_blk: Improve device creation with configfs")
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/null_blk/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..eba204b27785 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -66,7 +66,7 @@ struct nullb_page {
 #define NULLB_PAGE_FREE (MAP_SZ - 2)
 
 static LIST_HEAD(nullb_list);
-static struct mutex lock;
+static DEFINE_MUTEX(lock);
 static int null_major;
 static DEFINE_IDA(nullb_indexes);
 static struct blk_mq_tag_set tag_set;
@@ -2166,8 +2166,6 @@ static int __init null_init(void)
 	if (ret)
 		return ret;
 
-	mutex_init(&lock);
-
 	null_major = register_blkdev(0, "nullb");
 	if (null_major < 0) {
 		ret = null_major;
-- 
2.52.0

[PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

The file-scope lock mutex serializes access to global null_blk state,
including the nullb_list and device creation/removal. Rename it to
"nullb_global_lock" to make its purpose clear. No functional change.

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index eba204b27785..98f6935bb502 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -66,7 +66,7 @@ struct nullb_page {
 #define NULLB_PAGE_FREE (MAP_SZ - 2)
 
 static LIST_HEAD(nullb_list);
-static DEFINE_MUTEX(lock);
+static DEFINE_MUTEX(nullb_global_lock);
 static int null_major;
 static DEFINE_IDA(nullb_indexes);
 static struct blk_mq_tag_set tag_set;
@@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
 {
 	int ret;
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 
 	return ret;
 }
@@ -435,9 +435,9 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
 {
 	int ret;
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 
 	return ret;
 }
@@ -493,7 +493,7 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 		return ret;
 
 	ret = count;
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	if (!dev->power && newp) {
 		if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))
 			goto out;
@@ -516,7 +516,7 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 	}
 
 out:
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 	return ret;
 }
 
@@ -707,10 +707,10 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item)
 	struct nullb_device *dev = to_nullb_device(item);
 
 	if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
-		mutex_lock(&lock);
+		mutex_lock(&nullb_global_lock);
 		dev->power = false;
 		null_del_dev(dev->nullb);
-		mutex_unlock(&lock);
+		mutex_unlock(&nullb_global_lock);
 	}
 	nullb_del_fault_config(dev);
 	config_item_put(item);
@@ -2081,14 +2081,14 @@ static struct nullb *null_find_dev_by_name(const char *name)
 {
 	struct nullb *nullb = NULL, *nb;
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	list_for_each_entry(nb, &nullb_list, list) {
 		if (strcmp(nb->disk_name, name) == 0) {
 			nullb = nb;
 			break;
 		}
 	}
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 
 	return nullb;
 }
@@ -2102,9 +2102,9 @@ static int null_create_dev(void)
 	if (!dev)
 		return -ENOMEM;
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	ret = null_add_dev(dev);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 	if (ret) {
 		null_free_dev(dev);
 		return ret;
@@ -2200,17 +2200,17 @@ static void __exit null_exit(void)
 
 	unregister_blkdev(null_major, "nullb");
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_global_lock);
 	while (!list_empty(&nullb_list)) {
 		nullb = list_entry(nullb_list.next, struct nullb, list);
 		null_destroy_dev(nullb);
 	}
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_global_lock);
 
 	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
 
-	mutex_destroy(&lock);
+	mutex_destroy(&nullb_global_lock);
 }
 
 module_init(null_init);
-- 
2.52.0

[PATCH V2 3/6] null_blk: register configfs subsystem after creating default devices

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

In null_init(), configfs_register_subsystem() currently runs before
register_blkdev(), so when null_blk is built as a module, a racing mkdir()
+ poweron from userspace can reach null_add_dev() while null_major is still
0. __add_disk() then hits WARN_ON(disk->minors) (major=0 with minors!=0)
and fails:

[root@fedora ~]# [ 2366.521436] WARNING: block/genhd.c:476 at __add_disk+0x8a7/0xde0,
[ 2366.523552] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
[ 2366.529081] CPU: 26 UID: 0 PID: 1600 Comm: sh Not tainted 7.2.0-rc1+ #66 PREEMPT(full)
......
[ 2366.547251] Call Trace:
[ 2366.547575]  <TASK>
[ 2366.547831]  ? _raw_spin_lock+0x84/0xe0
[ 2366.548260]  add_disk_fwnode+0x114/0x560
[ 2366.548739]  null_add_dev+0x102d/0x1b80 [null_blk]
[ 2366.549310]  ? __pfx_null_add_dev+0x10/0x10 [null_blk]
[ 2366.549906]  ? mutex_lock+0xde/0x1c0
[ 2366.550361]  ? __pfx_mutex_lock+0x10/0x10
[ 2366.550827]  nullb_device_power_store+0x1e7/0x280 [null_blk]
[ 2366.551499]  ? __pfx_nullb_device_power_store+0x10/0x10 [null_blk]
[ 2366.552177]  ? __kmalloc_cache_noprof+0x1f5/0x470
[ 2366.552748]  ? configfs_write_iter+0x35c/0x4e0
[ 2366.553242]  configfs_write_iter+0x286/0x4e0
[ 2366.553787]  vfs_write+0x52d/0xd00
[ 2366.554169]  ? __pfx_vfs_write+0x10/0x10
[ 2366.554679]  ? __pfx___css_rstat_updated+0x10/0x10
[ 2366.555196]  ? fdget_pos+0x1cf/0x4c0
[ 2366.555649]  ksys_write+0xfc/0x1d0
......

Additionally, the err_dev path destroys all devices on nullb_list while
configfs is still registered. If a racing mkdir() + poweron puts a user
device on the list, null_destroy_dev()->null_free_dev() kfrees the user
device's nullb_device but /sys/kernel/config/nullb/<name> is still
reachable. Any userspace access to the item will trigger a UAF.

For simplicity, move configfs_register_subsystem() to the end to solve
the problems above. This also mirrors null_exit().

Fixes: 3bf2bd20734e ("nullb: add configfs interface")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 drivers/block/null_blk/main.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 98f6935bb502..024c89b36ddc 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2162,15 +2162,9 @@ static int __init null_init(void)
 	config_group_init(&nullb_subsys.su_group);
 	mutex_init(&nullb_subsys.su_mutex);
 
-	ret = configfs_register_subsystem(&nullb_subsys);
-	if (ret)
-		return ret;
-
 	null_major = register_blkdev(0, "nullb");
-	if (null_major < 0) {
-		ret = null_major;
-		goto err_conf;
-	}
+	if (null_major < 0)
+		return null_major;
 
 	for (i = 0; i < nr_devices; i++) {
 		ret = null_create_dev();
@@ -2178,6 +2172,10 @@ static int __init null_init(void)
 			goto err_dev;
 	}
 
+	ret = configfs_register_subsystem(&nullb_subsys);
+	if (ret)
+		goto err_dev;
+
 	pr_info("module loaded\n");
 	return 0;
 
@@ -2187,8 +2185,6 @@ static int __init null_init(void)
 		null_destroy_dev(nullb);
 	}
 	unregister_blkdev(null_major, "nullb");
-err_conf:
-	configfs_unregister_subsystem(&nullb_subsys);
 	return ret;
 }
 
-- 
2.52.0

[PATCH V2 4/6] null_blk: move unregister_blkdev() after destroying dev in null_exit()

[Zizhi Wo]

In null_exit(), unregister_blkdev() was called before the null_blk
instances were destroyed, which is inconsistent with the cleanup order in
null_init(). Move it after null_destroy_dev() so that teardown happens in
the reverse order of initialization.

No functional change intended.

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com>
---
 drivers/block/null_blk/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 024c89b36ddc..cab51301560e 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2194,8 +2194,6 @@ static void __exit null_exit(void)
 
 	configfs_unregister_subsystem(&nullb_subsys);
 
-	unregister_blkdev(null_major, "nullb");
-
 	mutex_lock(&nullb_global_lock);
 	while (!list_empty(&nullb_list)) {
 		nullb = list_entry(nullb_list.next, struct nullb, list);
@@ -2203,6 +2201,8 @@ static void __exit null_exit(void)
 	}
 	mutex_unlock(&nullb_global_lock);
 
+	unregister_blkdev(null_major, "nullb");
+
 	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
 
-- 
2.52.0

[PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
after apply_fn() returns. For attributes with an apply_fn (submit_queues,
poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.

configfs serializes writes via a per-open-file mutex (buffer->mutex), so
two threads writing to the same attribute through separate open file
descriptions run the store callback concurrently. The macro's write is
redundant and lockless, so a concurrent store's losing thread can overwrite
the winner's value after apply_fn set it, making dev->submit_queues
mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
falls back to a single queue.

Restructure the macro so that apply_fn attributes return directly after
apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
only changeable while not CONFIGURED and have no live hardware state to
mismatch.

Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 drivers/block/null_blk/main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index cab51301560e..e7555c47b671 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -360,13 +360,15 @@ 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;						\
-	if (apply_fn)							\
+	if (apply_fn) {							\
 		ret = apply_fn(dev, new_value);				\
-	else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) 	\
-		ret = -EBUSY;						\
-	if (ret < 0)							\
-		return ret;						\
-	dev->NAME = new_value;						\
+		if (ret < 0)						\
+			return ret;					\
+	} else {							\
+		if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))	\
+			return -EBUSY;					\
+		dev->NAME = new_value;					\
+	}								\
 	return count;							\
 }									\
 CONFIGFS_ATTR(nullb_device_, NAME);
-- 
2.52.0

[PATCH V2 6/6] null_blk: free global tag_set on init error path

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

If shared_tags is enabled, null_setup_tagset() allocates the global tag_set
via null_init_global_tag_set(). If device creation later fails, err_dev
destroys the default devices and calls unregister_blkdev(), but never frees
the global tag_set. Since module init failed, null_exit() is never invoked,
so the global tag_set's tags and maps are permanently leaked.

Free the global tag_set in err_dev, matching null_exit() which does
if (tag_set.ops) blk_mq_free_tag_set(&tag_set).

Fixes: 82f402fefa50 ("null_blk: add support for shared tags")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 drivers/block/null_blk/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index e7555c47b671..1e24c7e23524 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2187,6 +2187,8 @@ static int __init null_init(void)
 		null_destroy_dev(nullb);
 	}
 	unregister_blkdev(null_major, "nullb");
+	if (tag_set.ops)
+		blk_mq_free_tag_set(&tag_set);
 	return ret;
 }
 
-- 
2.52.0

Re: [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Zizhi Wo]

在 2026/7/7 10:55, Zizhi Wo 写道:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
> after apply_fn() returns. For attributes with an apply_fn (submit_queues,
> poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
> 
> configfs serializes writes via a per-open-file mutex (buffer->mutex), so
> two threads writing to the same attribute through separate open file
> descriptions run the store callback concurrently. The macro's write is
> redundant and lockless, so a concurrent store's losing thread can overwrite
> the winner's value after apply_fn set it, making dev->submit_queues
> mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
> falls back to a single queue.
> 
> Restructure the macro so that apply_fn attributes return directly after
> apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
> only changeable while not CONFIGURED and have no live hardware state to
> mismatch.
> 
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
>   drivers/block/null_blk/main.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index cab51301560e..e7555c47b671 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -360,13 +360,15 @@ 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;						\
> -	if (apply_fn)							\
> +	if (apply_fn) {							\
>   		ret = apply_fn(dev, new_value);				\
> -	else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) 	\
> -		ret = -EBUSY;						\
> -	if (ret < 0)							\
> -		return ret;						\
> -	dev->NAME = new_value;						\
> +		if (ret < 0)						\
> +			return ret;					\
> +	} else {							\
> +		if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))	\
> +			return -EBUSY;					\
> +		dev->NAME = new_value;					\
> +	}								\
>   	return count;							\
>   }									\
>   CONFIGFS_ATTR(nullb_device_, NAME);

Sorry for the noise. I missed that nullb_update_nr_hw_queues() returns
early without storing the value when !dev->nullb, so dropping the
macro's trailing write would silently discard pre-power-on
configuration. I'll add the update logic on the !dev->nullb path in v3.

Any further comments are welcome.

Thanks,
Zizhi Wo

Re: [PATCH V2 1/6] null_blk: use DEFINE_MUTEX for the file-scope mutex

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> In null_init(), mutex_init(&lock) currently happens after
> configfs_register_subsystem(), which exposes the nullb subsystem to
> userspace. A racing mkdir() into /sys/kernel/config/nullb/ can reach
> null_find_dev_by_name() -> mutex_lock(&lock) before the mutex is
> initialized, trigger warning:
> 
> [  123.137788] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
> [  123.137796] WARNING: kernel/locking/mutex.c:159 at mutex_lock+0x171/0x1c0, CPU#13: mkdir/1301
> [  123.140090] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4
> ......
> [  123.154926] Call Trace:
> [  123.155172]  <TASK>
> [  123.155419]  ? __pfx_mutex_lock+0x10/0x10
> [  123.156181]  ? __pfx__raw_spin_lock+0x10/0x10
> [  123.156571]  nullb_group_make_group+0x20/0x100 [null_blk]
> [  123.157011]  configfs_mkdir+0x47b/0xc70
> [  123.157337]  ? __pfx_configfs_mkdir+0x10/0x10
> [  123.157719]  ? may_create_dentry+0x242/0x2e0
> [  123.158061]  vfs_mkdir+0x2a9/0x6c0
> [  123.158352]  filename_mkdirat+0x3dc/0x500
> [  123.158710]  ? __pfx_filename_mkdirat+0x10/0x10
> [  123.159070]  ? strncpy_from_user+0x3a/0x1d0
> [  123.159413]  __x64_sys_mkdir+0x6b/0x90
> [  123.159760]  do_syscall_64+0xea/0x600
> 
> Replace the runtime mutex_init(&lock) with a static DEFINE_MUTEX(lock)
> declaration to fix this issue.
> 
> Fixes: 49c3b9266a71 ("block: null_blk: Improve device creation with configfs")
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> The file-scope lock mutex serializes access to global null_blk state,
> including the nullb_list and device creation/removal. Rename it to
> "nullb_global_lock" to make its purpose clear. No functional change.
> 
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
>  drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index eba204b27785..98f6935bb502 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -66,7 +66,7 @@ struct nullb_page {
>  #define NULLB_PAGE_FREE (MAP_SZ - 2)
>  
>  static LIST_HEAD(nullb_list);
> -static DEFINE_MUTEX(lock);
> +static DEFINE_MUTEX(nullb_global_lock);

Since this seem to protect only the device list, why not simply call this
nullb_list_lock ?

>  static int null_major;
>  static DEFINE_IDA(nullb_indexes);
>  static struct blk_mq_tag_set tag_set;
> @@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>  {
>  	int ret;
>  
> -	mutex_lock(&lock);
> +	mutex_lock(&nullb_global_lock);
>  	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
> -	mutex_unlock(&lock);
> +	mutex_unlock(&nullb_global_lock);

Nothing in nullb_update_nr_hw_queues() touches the device list. So it is very
odd that nullb_global_lock is used for serialization here instead of a
nullb_device mutex. If you change this, why not a prep patch to introduce such a
mutex ?

>  
>  	return ret;
>  }
> @@ -435,9 +435,9 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
>  {
>  	int ret;
>  
> -	mutex_lock(&lock);
> +	mutex_lock(&nullb_global_lock);
>  	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
> -	mutex_unlock(&lock);
> +	mutex_unlock(&nullb_global_lock);

Same here.

>  
>  	return ret;
>  }


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 3/6] null_blk: register configfs subsystem after creating default devices

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> In null_init(), configfs_register_subsystem() currently runs before
> register_blkdev(), so when null_blk is built as a module, a racing mkdir()
> + poweron from userspace can reach null_add_dev() while null_major is still
> 0. __add_disk() then hits WARN_ON(disk->minors) (major=0 with minors!=0)
> and fails:
> 
> [root@fedora ~]# [ 2366.521436] WARNING: block/genhd.c:476 at __add_disk+0x8a7/0xde0,
> [ 2366.523552] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
> [ 2366.529081] CPU: 26 UID: 0 PID: 1600 Comm: sh Not tainted 7.2.0-rc1+ #66 PREEMPT(full)
> ......
> [ 2366.547251] Call Trace:
> [ 2366.547575]  <TASK>
> [ 2366.547831]  ? _raw_spin_lock+0x84/0xe0
> [ 2366.548260]  add_disk_fwnode+0x114/0x560
> [ 2366.548739]  null_add_dev+0x102d/0x1b80 [null_blk]
> [ 2366.549310]  ? __pfx_null_add_dev+0x10/0x10 [null_blk]
> [ 2366.549906]  ? mutex_lock+0xde/0x1c0
> [ 2366.550361]  ? __pfx_mutex_lock+0x10/0x10
> [ 2366.550827]  nullb_device_power_store+0x1e7/0x280 [null_blk]
> [ 2366.551499]  ? __pfx_nullb_device_power_store+0x10/0x10 [null_blk]
> [ 2366.552177]  ? __kmalloc_cache_noprof+0x1f5/0x470
> [ 2366.552748]  ? configfs_write_iter+0x35c/0x4e0
> [ 2366.553242]  configfs_write_iter+0x286/0x4e0
> [ 2366.553787]  vfs_write+0x52d/0xd00
> [ 2366.554169]  ? __pfx_vfs_write+0x10/0x10
> [ 2366.554679]  ? __pfx___css_rstat_updated+0x10/0x10
> [ 2366.555196]  ? fdget_pos+0x1cf/0x4c0
> [ 2366.555649]  ksys_write+0xfc/0x1d0
> ......
> 
> Additionally, the err_dev path destroys all devices on nullb_list while
> configfs is still registered. If a racing mkdir() + poweron puts a user
> device on the list, null_destroy_dev()->null_free_dev() kfrees the user
> device's nullb_device but /sys/kernel/config/nullb/<name> is still
> reachable. Any userspace access to the item will trigger a UAF.
> 
> For simplicity, move configfs_register_subsystem() to the end to solve
> the problems above. This also mirrors null_exit().
> 
> Fixes: 3bf2bd20734e ("nullb: add configfs interface")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 4/6] null_blk: move unregister_blkdev() after destroying dev in null_exit()

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> In null_exit(), unregister_blkdev() was called before the null_blk

s/was/is

> instances were destroyed, which is inconsistent with the cleanup order in
> null_init(). Move it after null_destroy_dev() so that teardown happens in
> the reverse order of initialization.
> 
> No functional change intended.
> 
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
> after apply_fn() returns. For attributes with an apply_fn (submit_queues,
> poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
> 
> configfs serializes writes via a per-open-file mutex (buffer->mutex), so
> two threads writing to the same attribute through separate open file
> descriptions run the store callback concurrently. The macro's write is
> redundant and lockless, so a concurrent store's losing thread can overwrite
> the winner's value after apply_fn set it, making dev->submit_queues
> mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
> falls back to a single queue.
> 
> Restructure the macro so that apply_fn attributes return directly after
> apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
> only changeable while not CONFIGURED and have no live hardware state to
> mismatch.
> 
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 6/6] null_blk: free global tag_set on init error path

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> If shared_tags is enabled, null_setup_tagset() allocates the global tag_set
> via null_init_global_tag_set(). If device creation later fails, err_dev
> destroys the default devices and calls unregister_blkdev(), but never frees
> the global tag_set. Since module init failed, null_exit() is never invoked,
> so the global tag_set's tags and maps are permanently leaked.
> 
> Free the global tag_set in err_dev, matching null_exit() which does
> if (tag_set.ops) blk_mq_free_tag_set(&tag_set).
> 
> Fixes: 82f402fefa50 ("null_blk: add support for shared tags")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Zizhi Wo]

在 2026/7/7 12:16, Damien Le Moal 写道:
> On 7/7/26 11:55, Zizhi Wo wrote:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> The file-scope lock mutex serializes access to global null_blk state,
>> including the nullb_list and device creation/removal. Rename it to
>> "nullb_global_lock" to make its purpose clear. No functional change.
>>
>> Suggested-by: Bart Van Assche <bvanassche@acm.org>
>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>> ---
>>   drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index eba204b27785..98f6935bb502 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -66,7 +66,7 @@ struct nullb_page {
>>   #define NULLB_PAGE_FREE (MAP_SZ - 2)
>>   
>>   static LIST_HEAD(nullb_list);
>> -static DEFINE_MUTEX(lock);
>> +static DEFINE_MUTEX(nullb_global_lock);
> 
> Since this seem to protect only the device list, why not simply call this
> nullb_list_lock ?
> 
>>   static int null_major;
>>   static DEFINE_IDA(nullb_indexes);
>>   static struct blk_mq_tag_set tag_set;
>> @@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>>   {
>>   	int ret;
>>   
>> -	mutex_lock(&lock);
>> +	mutex_lock(&nullb_global_lock);
>>   	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
>> -	mutex_unlock(&lock);
>> +	mutex_unlock(&nullb_global_lock);
> 
> Nothing in nullb_update_nr_hw_queues() touches the device list. So it is very
> odd that nullb_global_lock is used for serialization here instead of a
> nullb_device mutex. If you change this, why not a prep patch to introduce such a
> mutex ?
> 

Thanks for pointing this out. The rename really belongs together with
the locking rework, so I'll drop this patch from v3 and fold the rename
into a separate series that splits the locking (introducing a per-
nullb_device mutex for the updates).

Thanks,
Zizhi Wo

>>   
>>   	return ret;
>>   }
>> @@ -435,9 +435,9 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
>>   {
>>   	int ret;
>>   
>> -	mutex_lock(&lock);
>> +	mutex_lock(&nullb_global_lock);
>>   	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
>> -	mutex_unlock(&lock);
>> +	mutex_unlock(&nullb_global_lock);
> 
> Same here.
> 
>>   
>>   	return ret;
>>   }
> 
> 

Re: [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Damien Le Moal]

On 7/7/26 15:24, Zizhi Wo wrote:
> 
> 
> 在 2026/7/7 12:16, Damien Le Moal 写道:
>> On 7/7/26 11:55, Zizhi Wo wrote:
>>> From: Zizhi Wo <wozizhi@huawei.com>
>>>
>>> The file-scope lock mutex serializes access to global null_blk state,
>>> including the nullb_list and device creation/removal. Rename it to
>>> "nullb_global_lock" to make its purpose clear. No functional change.
>>>
>>> Suggested-by: Bart Van Assche <bvanassche@acm.org>
>>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>>> ---
>>>   drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
>>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index eba204b27785..98f6935bb502 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -66,7 +66,7 @@ struct nullb_page {
>>>   #define NULLB_PAGE_FREE (MAP_SZ - 2)
>>>   
>>>   static LIST_HEAD(nullb_list);
>>> -static DEFINE_MUTEX(lock);
>>> +static DEFINE_MUTEX(nullb_global_lock);
>>
>> Since this seem to protect only the device list, why not simply call this
>> nullb_list_lock ?
>>
>>>   static int null_major;
>>>   static DEFINE_IDA(nullb_indexes);
>>>   static struct blk_mq_tag_set tag_set;
>>> @@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>>>   {
>>>   	int ret;
>>>   
>>> -	mutex_lock(&lock);
>>> +	mutex_lock(&nullb_global_lock);
>>>   	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
>>> -	mutex_unlock(&lock);
>>> +	mutex_unlock(&nullb_global_lock);
>>
>> Nothing in nullb_update_nr_hw_queues() touches the device list. So it is very
>> odd that nullb_global_lock is used for serialization here instead of a
>> nullb_device mutex. If you change this, why not a prep patch to introduce such a
>> mutex ?
>>
> 
> Thanks for pointing this out. The rename really belongs together with
> the locking rework, so I'll drop this patch from v3 and fold the rename
> into a separate series that splits the locking (introducing a per-
> nullb_device mutex for the updates).

Please carefully check if the use of the global lock in these function is not to
serialize with power on/off. The code is a little (uselessly) complicated in
that area and I may not be seeing something. But it seems that on/off control
could also use a device lock, with proper ordering: device lock first, then list
lock.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Zizhi Wo]

在 2026/7/7 14:28, Damien Le Moal 写道:
> On 7/7/26 15:24, Zizhi Wo wrote:
>>
>>
>> 在 2026/7/7 12:16, Damien Le Moal 写道:
>>> On 7/7/26 11:55, Zizhi Wo wrote:
>>>> From: Zizhi Wo <wozizhi@huawei.com>
>>>>
>>>> The file-scope lock mutex serializes access to global null_blk state,
>>>> including the nullb_list and device creation/removal. Rename it to
>>>> "nullb_global_lock" to make its purpose clear. No functional change.
>>>>
>>>> Suggested-by: Bart Van Assche <bvanassche@acm.org>
>>>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>>>> ---
>>>>    drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
>>>>    1 file changed, 16 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>> index eba204b27785..98f6935bb502 100644
>>>> --- a/drivers/block/null_blk/main.c
>>>> +++ b/drivers/block/null_blk/main.c
>>>> @@ -66,7 +66,7 @@ struct nullb_page {
>>>>    #define NULLB_PAGE_FREE (MAP_SZ - 2)
>>>>    
>>>>    static LIST_HEAD(nullb_list);
>>>> -static DEFINE_MUTEX(lock);
>>>> +static DEFINE_MUTEX(nullb_global_lock);
>>>
>>> Since this seem to protect only the device list, why not simply call this
>>> nullb_list_lock ?
>>>
>>>>    static int null_major;
>>>>    static DEFINE_IDA(nullb_indexes);
>>>>    static struct blk_mq_tag_set tag_set;
>>>> @@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>>>>    {
>>>>    	int ret;
>>>>    
>>>> -	mutex_lock(&lock);
>>>> +	mutex_lock(&nullb_global_lock);
>>>>    	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
>>>> -	mutex_unlock(&lock);
>>>> +	mutex_unlock(&nullb_global_lock);
>>>
>>> Nothing in nullb_update_nr_hw_queues() touches the device list. So it is very
>>> odd that nullb_global_lock is used for serialization here instead of a
>>> nullb_device mutex. If you change this, why not a prep patch to introduce such a
>>> mutex ?
>>>
>>
>> Thanks for pointing this out. The rename really belongs together with
>> the locking rework, so I'll drop this patch from v3 and fold the rename
>> into a separate series that splits the locking (introducing a per-
>> nullb_device mutex for the updates).
> 
> Please carefully check if the use of the global lock in these function is not to
> serialize with power on/off. The code is a little (uselessly) complicated in
> that area and I may not be seeing something. But it seems that on/off control
> could also use a device lock, with proper ordering: device lock first, then list
> lock.

Thanks for the suggestion. At a first glance this does look doable, and
I will check it carefully. :)

Thanks,
Zizhi Wo

> 
> 

Re: [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Nilay Shroff]

On 7/7/26 8:25 AM, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
> after apply_fn() returns. For attributes with an apply_fn (submit_queues,
> poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
> 
> configfs serializes writes via a per-open-file mutex (buffer->mutex), so
> two threads writing to the same attribute through separate open file
> descriptions run the store callback concurrently. The macro's write is
> redundant and lockless, so a concurrent store's losing thread can overwrite
> the winner's value after apply_fn set it, making dev->submit_queues
> mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
> falls back to a single queue.
> 
> Restructure the macro so that apply_fn attributes return directly after
> apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
> only changeable while not CONFIGURED and have no live hardware state to
> mismatch.
> 
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
>   drivers/block/null_blk/main.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index cab51301560e..e7555c47b671 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -360,13 +360,15 @@ 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;						\
> -	if (apply_fn)							\
> +	if (apply_fn) {							\
>   		ret = apply_fn(dev, new_value);				\
> -	else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) 	\
> -		ret = -EBUSY;						\
> -	if (ret < 0)							\
> -		return ret;						\
> -	dev->NAME = new_value;						\
> +		if (ret < 0)						\
> +			return ret;					\
> +	} else {							\
> +		if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))	\
> +			return -EBUSY;					\
> +		dev->NAME = new_value;					\
> +	}								\
>   	return count;							\
>   }									\
>   CONFIGFS_ATTR(nullb_device_, NAME);

Your patch correctly addresses the synchronization issue regarding the
apply_fn attributes under the nullb_list_lock. However, even with this
restructuring, the 'else' block remains vulnerable to concurrent
unmarked accesses to dev->NAME.

It seems when the device is not powered on, multiple threads can still
concurrently call the _show and _store callbacks on the same attribute
across separate open configfs file descriptions.

While this may be considered a benign data race, it can trigger KCSAN
splats and may allow the compiler to perform potentially unsafe optimization
heuristics (such as load/store tearing or merging). So I think we should
at-least mark those accesses using WRITE_ONCE() and READ_ONCE(). This also
help silence the KCSAN splat if it's configured.

Thanks,
--Nilay

Re: [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Zizhi Wo]

在 2026/7/7 15:33, Nilay Shroff 写道:
> On 7/7/26 8:25 AM, Zizhi Wo wrote:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
>> after apply_fn() returns. For attributes with an apply_fn (submit_queues,
>> poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
>>
>> configfs serializes writes via a per-open-file mutex (buffer->mutex), so
>> two threads writing to the same attribute through separate open file
>> descriptions run the store callback concurrently. The macro's write is
>> redundant and lockless, so a concurrent store's losing thread can 
>> overwrite
>> the winner's value after apply_fn set it, making dev->submit_queues
>> mismatch the hardware state. null_map_queues() then hits a 
>> WARN_ON_ONCE and
>> falls back to a single queue.
>>
>> Restructure the macro so that apply_fn attributes return directly after
>> apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
>> only changeable while not CONFIGURED and have no live hardware state to
>> mismatch.
>>
>> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after 
>> an instance has been configured")
>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>> ---
>>   drivers/block/null_blk/main.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/ 
>> main.c
>> index cab51301560e..e7555c47b671 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -360,13 +360,15 @@ 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;                        \
>> -    if (apply_fn)                            \
>> +    if (apply_fn) {                            \
>>           ret = apply_fn(dev, new_value);                \
>> -    else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))     \
>> -        ret = -EBUSY;                        \
>> -    if (ret < 0)                            \
>> -        return ret;                        \
>> -    dev->NAME = new_value;                        \
>> +        if (ret < 0)                        \
>> +            return ret;                    \
>> +    } else {                            \
>> +        if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))    \
>> +            return -EBUSY;                    \
>> +        dev->NAME = new_value;                    \
>> +    }                                \
>>       return count;                            \
>>   }                                    \
>>   CONFIGFS_ATTR(nullb_device_, NAME);
> 
> Your patch correctly addresses the synchronization issue regarding the
> apply_fn attributes under the nullb_list_lock. However, even with this
> restructuring, the 'else' block remains vulnerable to concurrent
> unmarked accesses to dev->NAME.
> 
> It seems when the device is not powered on, multiple threads can still
> concurrently call the _show and _store callbacks on the same attribute
> across separate open configfs file descriptions.
> 
> While this may be considered a benign data race, it can trigger KCSAN
> splats and may allow the compiler to perform potentially unsafe 
> optimization
> heuristics (such as load/store tearing or merging). So I think we should
> at-least mark those accesses using WRITE_ONCE() and READ_ONCE(). This also
> help silence the KCSAN splat if it's configured.
> 
> Thanks,
> --Nilay
> 

Thanks for catching this! The 'else' path can indeed be accessed
concurrently; I'd left it alone earlier assuming there was no real
impact, but from the angle you described, adding READ_ONCE()/
WRITE_ONCE() does make sense. I'll fix this in the next version.

Thanks,
Zizhi Wo