Re: [PATCHv2] md: clear MD_CLOSING when array_state_store() bails on interrupted lock

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling `restart_array()` with the `mddev->lock` spinlock held can sleep because it clears the disk read-only flag, which triggers a `GFP_KERNEL` memory allocation for a uevent.
--

--- Patch [1]: [PATCHv2] md: clear MD_CLOSING when array_state_store() bails on interrupted lock ---
commit 3ae9b81fbeee45b412994652c2c3b3ac1b35d177
Author: Jack Wang <[email protected]>

md: clear MD_CLOSING when array_state_store() bails on interrupted lock

This commit ensures that MD_CLOSING is correctly cleared if
mutex_lock_interruptible() fails while writing to array_state. It introduces
a common exit label and a boolean flag to track whether the flag was actually
set by the current thread.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..7b7a4d9259079 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -4696,7 +4698,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
>  	}
>  	err = mddev_lock(mddev);
>  	if (err)
> -		return err;
> +		goto out_clear_closing;

[Severity: High]
This is a pre-existing issue, but just above this code in array_state_store(),
there is a fast path that can sleep while holding a spinlock.

If a privileged user sets the md device to read-only using
blockdev --setro /dev/md0 and then writes 'active' to
/sys/block/md0/md/array_state, the code acquires the mddev->lock spinlock
and calls restart_array():

drivers/md/md.c:array_state_store() {
    ...
    spin_lock(&mddev->lock);
    if (st == active) {
        restart_array(mddev);
    ...
}

restart_array() unconditionally calls set_disk_ro() to ensure the disk is
read-write:

drivers/md/md.c:restart_array() {
    ...
    set_disk_ro(disk, 0);
    ...
}

Because the disk was set to read-only, set_disk_ro() clears the flag and
triggers a uevent:

block/genhd.c:set_disk_ro() {
    ...
    set_disk_ro_uevent(disk, read_only);
    ...
}

This calls into kobject_uevent_env(), which performs an unconditional
GFP_KERNEL allocation:

lib/kobject_uevent.c:kobject_uevent_env() {
    ...
    devpath = kobject_get_path(kobj, GFP_KERNEL);
    ...
}

Can this sequence cause a sleep-in-atomic bug and potentially lead to a
deadlock or panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1