Re: [PATCH v4] md: remove legacy async del_gendisk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: race between md_alloc and mddev_put causes duplicate sysfs warnings
--

commit 6f4e37981f391fc56de7da8a4b3ba68d6d5786d3
Author: Li Youhong <liyouhong@kylinos.cn>
Subject: md: remove legacy async del_gendisk

This patch removes the legacy async gendisk deletion path, making
del_gendisk() synchronous from mddev_unlock() and the final
kobject_put(). It also removes del_work and the flush_workqueue()
in md_alloc().

Closes: https://syzkaller.appspot.com/bug?extid=68e1f51046d68329940f

> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c

[ ... ]

>  void mddev_put(struct mddev *mddev)
>  {
> +	bool complete_delete;
> +
>  	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
>  		return;
>  
> -	__mddev_put(mddev);
> +	complete_delete = __mddev_put(mddev);
> +	if (complete_delete)
> +		mddev_unlist(mddev);
>  	spin_unlock(&all_mddevs_lock);
> +	if (complete_delete)
> +		mddev_complete_delete(mddev);
>  }

[Severity: High]
Does unlisting the dying mddev from all_mddevs before its gendisk is fully
deleted make the old name prematurely invisible to a concurrent md_alloc()?

If a process triggers mddev_put() (e.g. by closing an unconfigured /dev/mdXX),
the array is removed from all_mddevs before the synchronous del_gendisk()
is called.

[ ... ]

> @@ -6327,12 +6334,6 @@ struct mddev *md_alloc(dev_t dev, char *name)
>  	int unit;
>  	int error;
>  
> -	/*
> -	 * Wait for any previous instance of this device to be completely
> -	 * removed (mddev_delayed_delete).
> -	 */
> -	flush_workqueue(md_misc_wq);
> -
>  	mutex_lock(&disks_mutex);
>  	mddev = mddev_alloc(dev);

[Severity: High]
Since flush_workqueue(md_misc_wq) is removed, could a concurrent md_alloc()
proceed and fail to find the unlisted mddev?

If md_alloc() starts while mddev_put() is deleting a gendisk with the same
name, it won't find the old array in all_mddevs.

It would then allocate a new gendisk with the same name and call add_disk(),
which collides with the dying gendisk.

This leads to a -EEXIST failure in add_disk() and a kernel warning
for duplicate sysfs filenames. On systems with panic_on_warn set, this could
cause a crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002065642.1726950-1-dayou5941@163.com?part=1