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

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] md: Use-After-Free in md_notify_reboot and md_exit due to unsafe list_for_each_entry_safe
- [High] md: Use-After-Free in md_seq_show when mddev_get fails
- [High] md: sysfs self-deadlock in md_attr_store due to synchronous kobject_del
--

commit 882f6ccb12f02b986de958463cf3afb9ffc46b20
Author: Li Youhong [off-list ref]

md: remove legacy async del_gendisk

This commit removes the legacy async del_gendisk path to resolve a lockdep
cycle between major_names_lock, md_misc_wq, and reconfig_mutex. Deletions
are now done synchronously in mddev_unlock() and mddev_put().

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

[ ... ]

 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);
 	spin_unlock(&all_mddevs_lock);
+	if (complete_delete)
+		mddev_complete_delete(mddev);
 }

[Severity: High]
Could synchronous kobject_del() on the active path in drivers/md/md.c:mddev_put()
cause a self-deadlock when triggered from a sysfs handler like md_attr_store()?

Writing 'stop' to /sys/block/mdX/md/array_state executes md_attr_store() under
kernfs active protection. The handler calls do_md_stop(), marking the array as
deleted, and then calls mddev_put().

Since mddev_put() now synchronously invokes mddev_complete_delete() ->
kobject_del(), kobject_del() will call kernfs_remove() and block in
kernfs_drain() waiting for the active protection to drop. The calling thread
is holding the active protection itself, causing a deadlock regression.

[ ... ]

[Severity: High]
Can this sequence in drivers/md/md.c:md_seq_show() lead to a use-after-free
regression when mddev_get() fails?

If mddev_get() fails (because MD_DELETED is set), the current iterator v is
unpinned. The code then calls md_seq_flush_delete(seq), which temporarily
drops all_mddevs_lock to perform delayed deletion of a previous node.

During this unlocked window, the unpinned current node v could be fully
deleted and freed by a concurrent thread. When md_seq_flush_delete() reacquires
the lock and md_seq_show() returns 0, seq_read() will call md_seq_next(),
which dereferences the now-freed v pointer.

[ ... ]

[Severity: High]
Does dropping all_mddevs_lock inside list_for_each_entry_safe() in
drivers/md/md.c:md_notify_reboot() expose tmp to concurrent deletion?

Since list_for_each_entry_safe() caches the next pointer into tmp before
executing the loop body, tmp is unpinned and unprotected during the unlocked
window. Because mddev_put() now performs synchronous deletion, a concurrent
array teardown could drop the last reference to tmp and free its memory. When
the lock is reacquired, the loop advances by reading tmp->all_mddevs.next,
which dereferences freed memory.

[ ... ]

[Severity: High]
Similarly, does dropping the lock here in drivers/md/md.c:md_exit() while
holding an unpinned tmp risk list corruption on module exit?

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