[PATCH] md: make mddev_get() fail until md_alloc() has finished

[PATCH] md: make mddev_get() fail until md_alloc() has finished

[Abd-Alrhman Masalkhi]

md_alloc() makes the disk visible with add_disk() before it adds
mddev->kobj. In between, md_open() can take a reference with
mddev_get() and send ioctls to an mddev whose kobject is not set up
yet, for example ADD_NEW_DISK, which adds the rdev kobject under
mddev->kobj.

If kobject_add() then fails, md_alloc() drops its own reference and
expects mddev_put() to delete the mddev. A reference taken in the
window keeps it alive instead, and if a command such as SET_ARRAY_INFO
sets ->ctime, the final __mddev_put() keeps the mddev for good.

Add mddev->sysfs_live, set under all_mddevs_lock after kobject_add()
succeeds and the sysfs handles are cached. Make mddev_get() fail
while it is not set. mddev_get() already runs under all_mddevs_lock,
so whoever gets a reference sees a fully set up mddev.

Also, send the KOBJ_ADD uevent only after mddev->sysfs_live is set,
so that user space reacting to it finds a usable mddev.

An open that races with md_alloc() now fails with -ENODEV, and a
sysfs read in md/ before the flag is set fails with -EBUSY.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261029202209.1422567-1-abd.masalkhi@gmail.com?part=4
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/md.c | 10 ++++++++--
 drivers/md/md.h |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 154ee5a65cb7..20acd1e25e9e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -627,7 +627,7 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
 {
 	lockdep_assert_held(&all_mddevs_lock);
 
-	if (test_bit(MD_DELETED, &mddev->flags))
+	if (test_bit(MD_DELETED, &mddev->flags) || !mddev->sysfs_live)
 		return NULL;
 	atomic_inc(&mddev->active);
 	return mddev;
@@ -6403,10 +6403,16 @@ struct mddev *md_alloc(dev_t dev, char *name)
 		return ERR_PTR(error);
 	}
 
-	kobject_uevent(&mddev->kobj, KOBJ_ADD);
 	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
 	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
+
+	spin_lock(&all_mddevs_lock);
+	mddev->sysfs_live = true;
+	spin_unlock(&all_mddevs_lock);
+
+	kobject_uevent(&mddev->kobj, KOBJ_ADD);
 	mutex_unlock(&disks_mutex);
+
 	return mddev;
 
 out_put_disk:
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 6440da292105..5e026307ca64 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -417,6 +417,10 @@ struct mddev {
 	struct mutex			suspend_mutex;
 	struct percpu_ref		active_io;
 	int				ro;
+	bool				sysfs_live; /* set under all_mddevs_lock
+						     * after kobject_add()
+						     * succeeds, never cleared
+						     */
 	int				sysfs_active; /* set when sysfs deletes
 						       * are happening, so run/
 						       * takeover/stop are not safe
-- 
2.43.0

Re: [PATCH] md: make mddev_get() fail until md_alloc() has finished

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] md: premature KOBJ_ADD uevent in add_disk() while mddev_get()
  fails
- [Low] md: stale reconfig_mutex lock and comment in md_alloc()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] md: kernfs_node memory leak in mddev_unlock() [1]
--

commit 2ddaaecbff58458e8c38353164498300daf7d131
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>

md: make mddev_get() fail until md_alloc() has finished

This commit adds an mddev->sysfs_live flag set under all_mddevs_lock after
kobject_add() succeeds in md_alloc(), and updates mddev_get() to fail until
that flag is set.

Link: https://sashiko.dev/#/patchset/20261029202209.1422567-1-abd.masalkhi@gmail.com?part=4

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 03e35386a8028..47118e9d1a656 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -641,7 +641,7 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
>  {
>  	lockdep_assert_held(&all_mddevs_lock);
>  
> -	if (test_bit(MD_DELETED, &mddev->flags))
> +	if (test_bit(MD_DELETED, &mddev->flags) || !mddev->sysfs_live)
>  		return NULL;
>  	atomic_inc(&mddev->active);
>  	return mddev;
> @@ -6432,10 +6432,16 @@ struct mddev *md_alloc(dev_t dev, char *name)
>  		return ERR_PTR(error);
>  	}
>  
> -	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>  	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
>  	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");

[Severity: Medium]
This problem wasn't introduced by this patch, but when
legacy_async_del_gendisk is disabled, does mddev_unlock() leak
mddev->sysfs_state and mddev->sysfs_level [1]?

md_kobj_release() only calls sysfs_put() on mddev->sysfs_state and
mddev->sysfs_level when legacy_async_del_gendisk is true, and the
!legacy_async_del_gendisk teardown path in mddev_unlock() deletes
mddev->kobj and mddev->gendisk without releasing those kernfs_node
references:

drivers/md/md.c:md_kobj_release() {
	if (legacy_async_del_gendisk) {
		if (mddev->sysfs_state)
			sysfs_put(mddev->sysfs_state);
		if (mddev->sysfs_level)
			sysfs_put(mddev->sysfs_level);
		del_gendisk(mddev->gendisk);
	}
	put_disk(mddev->gendisk);
}

> +
> +	spin_lock(&all_mddevs_lock);
> +	mddev->sysfs_live = true;
> +	spin_unlock(&all_mddevs_lock);
> +
> +	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>  	mutex_unlock(&disks_mutex);
> +
>  	return mddev;
>  
>  out_put_disk:

[Severity: Medium]
Does making mddev_get() fail until mddev->sysfs_live is set cause a
regression where udevd or concurrent openers fail with -ENODEV in md_open()
and -EBUSY on md/ sysfs attributes right after add_disk() announces the
disk?

kobject_uevent(&mddev->kobj, KOBJ_ADD) is dropped by dev_uevent_filter()
because mddev->kobj has ktype &md_ktype rather than &device_ktype.  The
KOBJ_ADD uevent that actually reaches user space is sent earlier inside
add_disk() -> add_disk_final() -> disk_uevent(disk, KOBJ_ADD), right after
bdev_add() publishes the block device inode while mddev->sysfs_live is
still false:

CPU0                              CPU1 (udevd / opener)
md_alloc()
  add_disk(disk)
    add_disk_final(disk)
      bdev_add(...)
      disk_uevent(disk, KOBJ_ADD)
                                  blkdev_get_no_open()
                                  md_open()
                                    mddev_find()
                                      mddev_get() -> NULL  <-- -ENODEV
  kobject_add(&mddev->kobj, ...)
  mddev->sysfs_live = true

[Severity: Low]
Since mddev_get() now returns NULL until mddev->sysfs_live is set after
mddev_unlock(mddev), md_open() can no longer succeed while reconfig_mutex
is held across add_disk() and kobject_add() from commit d1472ca2ca5d ("md:
don't hand out the array before md_alloc() has added mddev->kobj").

drivers/md/md.c:md_alloc() {
	/*
	 * add_disk() makes the array openable before mddev->kobj exists.
	 * Hold reconfig_mutex until kobject_add() is done so that ioctls
	 * issued in between wait instead of using an uninitialised kobj.
	 */
	mddev_lock_nointr(mddev);
	error = add_disk(disk);
    ...
	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
	mddev_unlock(mddev);
}

Does this make the comment and reconfig_mutex critical section in
md_alloc() obsolete, as well as the commit message's first paragraph
claiming that ioctls such as ADD_NEW_DISK can run before kobject_add()
completes?

[1] https://sashiko.dev/#/bug/linux-460ea5ed-96ea-49a0-8383-cfa84f28520b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112114958.2244-1-abd.masalkhi@gmail.com?part=1

Re: [PATCH] md: make mddev_get() fail until md_alloc() has finished

[Abd-Alrhman Masalkhi]

Hi Kuai,

On Thu, Nov 12, 2026 at 12:13 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Medium] md: premature KOBJ_ADD uevent in add_disk() while mddev_get()
>   fails
> - [Low] md: stale reconfig_mutex lock and comment in md_alloc()
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [Medium] md: kernfs_node memory leak in mddev_unlock() [1]
> --
>
> commit 2ddaaecbff58458e8c38353164498300daf7d131
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md: make mddev_get() fail until md_alloc() has finished
>
> This commit adds an mddev->sysfs_live flag set under all_mddevs_lock after
> kobject_add() succeeds in md_alloc(), and updates mddev_get() to fail until
> that flag is set.
>
> Link: https://sashiko.dev/#/patchset/20261029202209.1422567-1-abd.masalkhi@gmail.com?part=4
>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 03e35386a8028..47118e9d1a656 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -641,7 +641,7 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
>>  {
>>  	lockdep_assert_held(&all_mddevs_lock);
>>  
>> -	if (test_bit(MD_DELETED, &mddev->flags))
>> +	if (test_bit(MD_DELETED, &mddev->flags) || !mddev->sysfs_live)
>>  		return NULL;
>>  	atomic_inc(&mddev->active);
>>  	return mddev;
>> @@ -6432,10 +6432,16 @@ struct mddev *md_alloc(dev_t dev, char *name)
>>  		return ERR_PTR(error);
>>  	}
>>  
>> -	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>  	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
>>  	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
>
> [Severity: Medium]
> This problem wasn't introduced by this patch, but when
> legacy_async_del_gendisk is disabled, does mddev_unlock() leak
> mddev->sysfs_state and mddev->sysfs_level [1]?
>
> md_kobj_release() only calls sysfs_put() on mddev->sysfs_state and
> mddev->sysfs_level when legacy_async_del_gendisk is true, and the
> !legacy_async_del_gendisk teardown path in mddev_unlock() deletes
> mddev->kobj and mddev->gendisk without releasing those kernfs_node
> references:
>
> drivers/md/md.c:md_kobj_release() {
> 	if (legacy_async_del_gendisk) {
> 		if (mddev->sysfs_state)
> 			sysfs_put(mddev->sysfs_state);
> 		if (mddev->sysfs_level)
> 			sysfs_put(mddev->sysfs_level);
> 		del_gendisk(mddev->gendisk);
> 	}
> 	put_disk(mddev->gendisk);
> }
>
>> +
>> +	spin_lock(&all_mddevs_lock);
>> +	mddev->sysfs_live = true;
>> +	spin_unlock(&all_mddevs_lock);
>> +
>> +	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>  	mutex_unlock(&disks_mutex);
>> +
>>  	return mddev;
>>  
>>  out_put_disk:
>
> [Severity: Medium]
> Does making mddev_get() fail until mddev->sysfs_live is set cause a
> regression where udevd or concurrent openers fail with -ENODEV in md_open()
> and -EBUSY on md/ sysfs attributes right after add_disk() announces the
> disk?
>
> kobject_uevent(&mddev->kobj, KOBJ_ADD) is dropped by dev_uevent_filter()
> because mddev->kobj has ktype &md_ktype rather than &device_ktype.  The
> KOBJ_ADD uevent that actually reaches user space is sent earlier inside
> add_disk() -> add_disk_final() -> disk_uevent(disk, KOBJ_ADD), right after
> bdev_add() publishes the block device inode while mddev->sysfs_live is
> still false:
>
> CPU0                              CPU1 (udevd / opener)
> md_alloc()
>   add_disk(disk)
>     add_disk_final(disk)
>       bdev_add(...)
>       disk_uevent(disk, KOBJ_ADD)
>                                   blkdev_get_no_open()
>                                   md_open()
>                                     mddev_find()
>                                       mddev_get() -> NULL  <-- -ENODEV
>   kobject_add(&mddev->kobj, ...)
>   mddev->sysfs_live = true
>

I missed this case. Do you have a suggestion for a better fix, or should
I just drop this patch?

> [Severity: Low]
> Since mddev_get() now returns NULL until mddev->sysfs_live is set after
> mddev_unlock(mddev), md_open() can no longer succeed while reconfig_mutex
> is held across add_disk() and kobject_add() from commit d1472ca2ca5d ("md:
> don't hand out the array before md_alloc() has added mddev->kobj").
>
> drivers/md/md.c:md_alloc() {
> 	/*
> 	 * add_disk() makes the array openable before mddev->kobj exists.
> 	 * Hold reconfig_mutex until kobject_add() is done so that ioctls
> 	 * issued in between wait instead of using an uninitialised kobj.
> 	 */
> 	mddev_lock_nointr(mddev);
> 	error = add_disk(disk);
>     ...
> 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
> 	mddev_unlock(mddev);
> }
>
> Does this make the comment and reconfig_mutex critical section in
> md_alloc() obsolete, as well as the commit message's first paragraph
> claiming that ioctls such as ADD_NEW_DISK can run before kobject_add()
> completes?
>
> [1] https://sashiko.dev/#/bug/linux-460ea5ed-96ea-49a0-8383-cfa84f28520b
>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261112114958.2244-1-abd.masalkhi@gmail.com?part=1

-- 
Best Regards,
Abd-Alrhman