Re: [PATCH v2] md: don't hand out the array before md_alloc() has added mddev->kobj

["yu kuai" <yukuai@fygo.io>]

Hi,

在 2026/10/6 20:29, Yogesh Gaur 写道:
> md_alloc() publishes the gendisk before it is done setting the mddev up:
>
> 	disk->private_data = mddev;
> 	...
> 	error = add_disk(disk);
> 	if (error)
> 		goto out_put_disk;
>
> 	kobject_init(&mddev->kobj, &md_ktype);
> 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
>
> add_disk() makes /dev/mdN openable.  mddev comes from mddev_alloc() and
> is zeroed, so anything that gets in between add_disk() and kobject_add()
> sees an mddev->kobj that has never been through kobject_init() - no
> ktype, no kref, state_initialized clear.
>
> syzbot opens the array in that window and issues ADD_NEW_DISK:
>
>    kobject: '(null)' (ffff8880120640f0): is not initialized, yet kobject_get() is being called.
>    WARNING: lib/kobject.c:642 at kobject_add_internal+0xea/0xcd0 lib/kobject.c:225
>     kobject_add_varg lib/kobject.c:374 [inline]
>     kobject_add+0x163/0x240 lib/kobject.c:426
>     bind_rdev_to_array+0x80c/0xdd0 drivers/md/md.c:2621
>     md_add_new_disk+0xe3b/0x1850 drivers/md/md.c:7684
>     md_ioctl+0x200a/0x2610 drivers/md/md.c:8499
>
> bind_rdev_to_array() is not the only way in.  md_run() calls
> sysfs_create_group(&mddev->kobj, &md_redundancy_group), and
> internal_create_group() has its own WARN_ON(!kobj->sd), so RUN_ARRAY in
> the same window warns too.
>
> Every ioctl that can reach mddev->kobj runs under reconfig_mutex, so
> hold it across add_disk() and kobject_add().  An ioctl issued in the
> window now waits for md_alloc() to finish instead of seeing a
> half-built mddev, and opening the array still succeeds.  The ioctls
> md_ioctl() serves without the mutex only read array state and return
> -ENODEV on an array with no disks.
>
> Hoisting kobject_init() above add_disk() is not an option: commit
> ca39f7502425 ("md: fix mddev->kobj lifetime") moved it below add_disk()
> so that md_alloc()'s error paths, which free the mddev directly, never
> see an initialised kobject.
>
> Fixes: ca39f7502425 ("md: fix mddev->kobj lifetime")
> Reported-by: syzbot+95eeb4ada2349a2170ea@syzkaller.appspotmail.com

Reported-by should always follow with a link to the report, and the following
tag and applied:

Closes: https://lore.kernel.org/all/6aaf3284.514bccd6.255447.0003.GAE@google.com/

> Suggested-by: Yu Kuai <yukuai@fygo.io>
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
> ---
> v2: Hold reconfig_mutex across add_disk() and kobject_add() instead of
>      refusing the open in md_open(), per Yu Kuai's review: v1 made an
>      open that raced with md_alloc() fail.
> v1: https://lore.kernel.org/all/20261004055712.1293-1-yogeshgaur.83@gmail.com/
>
> Built with W=1 only, fix has not been runtime-tested.
>
>   drivers/md/md.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 67108c397c5a..85439f27d943 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5906,12 +5906,22 @@ struct mddev *md_alloc(dev_t dev, char *name)
>   
>   	disk->events |= DISK_EVENT_MEDIA_CHANGE;
>   	mddev->gendisk = disk;
> +
> +	/*
> +	 * 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);
> -	if (error)
> +	if (error) {
> +		mddev_unlock(mddev);
>   		goto out_put_disk;
> +	}
>   
>   	kobject_init(&mddev->kobj, &md_ktype);
>   	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
> +	mddev_unlock(mddev);
>   	if (error) {
>   		/*
>   		 * The disk is already live at this point.  Clear the hold flag

-- 
Thanks,
Kuai