[PATCH 0/3] null_blk: fix mutex initialization and configfs teardown race

[PATCH 0/3] null_blk: fix mutex initialization and configfs teardown race

[Zizhi Wo]

This series fixes a few simple issues in null_blk.

Patches 1-2 fix a problem where the null_blk mutex was left uninitialized.
The fix for the first issue was originally sent as a separate patch[1].
Following Bart's suggestion, it now use DEFINE_MUTEX() and the lock has
been renamed to a more descriptive name.

Patch 3 fixes concurrency issues around configfs registration.

See the individual patch descriptions for details.

[1] https://lore.kernel.org/all/20260704092323.748772-1-wozizhi@huaweicloud.com/

Zizhi Wo (3):
  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

 drivers/block/null_blk/main.c | 50 +++++++++++++++--------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

-- 
2.52.0

[PATCH 1/3] 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>
---
 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

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

[Bart Van Assche]

On 7/6/26 5:35 AM, Zizhi Wo wrote:
> [ ... ]
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

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

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

The file-scope lock mutex mainly serializes access to the global nullb_list
and related device setup. Rename it to "nullb_list_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..a775e70e16fc 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_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_list_lock);
 	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_list_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_list_lock);
 	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_list_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_list_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_list_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_list_lock);
 		dev->power = false;
 		null_del_dev(dev->nullb);
-		mutex_unlock(&lock);
+		mutex_unlock(&nullb_list_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_list_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_list_lock);
 
 	return nullb;
 }
@@ -2102,9 +2102,9 @@ static int null_create_dev(void)
 	if (!dev)
 		return -ENOMEM;
 
-	mutex_lock(&lock);
+	mutex_lock(&nullb_list_lock);
 	ret = null_add_dev(dev);
-	mutex_unlock(&lock);
+	mutex_unlock(&nullb_list_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_list_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_list_lock);
 
 	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
 
-	mutex_destroy(&lock);
+	mutex_destroy(&nullb_list_lock);
 }
 
 module_init(null_init);
-- 
2.52.0

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

[Bart Van Assche]

On 7/6/26 5:35 AM, Zizhi Wo wrote:
> The file-scope lock mutex mainly serializes access to the global nullb_list
> and related device setup. Rename it to "nullb_list_lock" to make its
> purpose clear. No functional change.

 From commit a2db328b0839 ("null_blk: fix null-ptr-dereference while
configuring 'power' and 'submit_queues'"):

     Writing 'power' and 'submit_queues' concurrently will trigger kernel
     panic:
     [ ... ]
     Fix this problem by resuing the global mutex to protect
     nullb_device_power_store() and nullb_update_nr_hw_queues() from 
configfs.

This makes it clear that the "lock" mutex has a broader purpose than
only protecting nullb_list. Hence, the name nullb_list_lock is
confusing. How about using the name "nullb_lock"?

Thanks,

Bart.

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

[Zizhi Wo]

在 2026/7/6 21:12, Bart Van Assche 写道:
> On 7/6/26 5:35 AM, Zizhi Wo wrote:
>> The file-scope lock mutex mainly serializes access to the global 
>> nullb_list
>> and related device setup. Rename it to "nullb_list_lock" to make its
>> purpose clear. No functional change.
> 
>  From commit a2db328b0839 ("null_blk: fix null-ptr-dereference while
> configuring 'power' and 'submit_queues'"):
> 
>      Writing 'power' and 'submit_queues' concurrently will trigger kernel
>      panic:
>      [ ... ]
>      Fix this problem by resuing the global mutex to protect
>      nullb_device_power_store() and nullb_update_nr_hw_queues() from 
> configfs.
> 
> This makes it clear that the "lock" mutex has a broader purpose than
> only protecting nullb_list. Hence, the name nullb_list_lock is
> confusing. How about using the name "nullb_lock"?
> 
> Thanks,
> 
> Bart.

Thanks for the reminder! Agreed that "nullb_list_lock" is too narrow
since it also serializes nullb_device_power_store() and
nullb_update_nr_hw_queues().

However, I'm a bit worried that "nullb_lock" is easy to confuse with the
per-device spinlock "nullb->lock"... They are only one "->" apart?

Would "nullb_global_lock" (or "nullb_config_lock") work better? It
reflects the broader purpose and avoids clashing with nullb->lock.

Thanks,
Zizhi Wo

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

[Bart Van Assche]

On 7/6/26 6:21 AM, Zizhi Wo wrote:
> Thanks for the reminder! Agreed that "nullb_list_lock" is too narrow
> since it also serializes nullb_device_power_store() and
> nullb_update_nr_hw_queues().
> 
> However, I'm a bit worried that "nullb_lock" is easy to confuse with the
> per-device spinlock "nullb->lock"... They are only one "->" apart?
> 
> Would "nullb_global_lock" (or "nullb_config_lock") work better? It
> reflects the broader purpose and avoids clashing with nullb->lock.

Not sure what others prefer but nullb_global_lock sounds good to me.

Thanks,

Bart.

[PATCH 3/3] 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 a775e70e16fc..9800049b3c3f 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

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

[Bart Van Assche]

On 7/6/26 5:35 AM, Zizhi Wo wrote:
> @@ -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;
>   }

Please make null_exit() consistent with the cleanup path in null_init().
null_exit() calls unregister_blkdev() before it destroys the null_blk
instances. That doesn't look right to me.

Thanks,

Bart.

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

[Zizhi Wo]

在 2026/7/6 21:17, Bart Van Assche 写道:
> On 7/6/26 5:35 AM, Zizhi Wo wrote:
>> @@ -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;
>>   }
> 
> Please make null_exit() consistent with the cleanup path in null_init().
> null_exit() calls unregister_blkdev() before it destroys the null_blk
> instances. That doesn't look right to me.
> 
> Thanks,
> 
> Bart.
> 
> 

Thanks for pointing this out, I hadn't noticed that. I'll add this
change in the next version.

Thanks,
Zizhi Wo