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

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

[Jack Wang]

From: Jack Wang <jinpu.wang@cloud.ionos.com>

Writing "clear", "readonly", "inactive" or "read_auto" to array_state
calls mddev_set_closing_and_sync_blockdev(), which sets MD_CLOSING so the
array cannot be reopened while it is being torn down. The flag is only
cleared again at the tail of array_state_store() (for readonly, read_auto,
inactive, or the failed-clear case).

Between setting the flag and reaching that tail there is an early return:

	err = mddev_lock(mddev);
	if (err)
		return err;

mddev_lock() is mutex_lock_interruptible() on reconfig_mutex. If the
writing task is signalled while blocked there - easy to hit when the mutex
is held for a long time by a running resync/recovery or reshape - it
returns -EINTR and array_state_store() returns with MD_CLOSING still set.
do_md_stop()/md_set_readonly() never ran, so the array keeps working
internally, but every subsequent md_open() now returns -ENODEV for all
callers: the device still shows up in /proc/mdstat and sysfs is fully
populated, yet it cannot be opened and cannot be recovered without a
reboot (the remaining clear paths either need the device open or re-enter
test_and_set_bit(MD_CLOSING) and bail with -EBUSY before the clear).

Route the interrupted-lock exit through the existing MD_CLOSING clearing
logic instead of returning directly, mirroring the goto-out pattern
md_ioctl() already uses for the same flag. mddev_unlock() is correctly
skipped since the lock was never acquired.

Fixes: 99b902ac1725 ("md: sync blockdev before stopping raid or setting readonly")
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/md/md.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..c4de2053593a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4696,7 +4696,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;
 
 	switch (st) {
 	case inactive:
@@ -4769,6 +4769,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 	}
 	mddev_unlock(mddev);
 
+out_clear_closing:
 	if (st == readonly || st == read_auto || st == inactive ||
 	    (err && st == clear))
 		clear_bit(MD_CLOSING, &mddev->flags);
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional clearing of MD_CLOSING on the error path can clear a flag owned by a concurrent thread, exposing the array to use-after-free races during teardown.
--

--- Patch [1]: [PATCH] md: clear MD_CLOSING when array_state_store() bails on interrupted lock ---
commit 919b922dd46da90c5c7e1a2b46106db8f5e8ffbf
Author: Jack Wang <jinpu.wang@cloud.ionos.com>

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

This commit addresses a bug where an interrupted lock in array_state_store()
leaves MD_CLOSING set, preventing the array from being reopened. It modifies
the error path to jump to the existing cleanup logic to clear the flag.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..c4de2053593a0 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4696,7 +4696,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]
Does this unconditionally clear MD_CLOSING even if this thread didn't set it?

If Thread A initiates array teardown (e.g., via md_ioctl(STOP_ARRAY)), it sets
MD_CLOSING, takes reconfig_mutex, and sets mddev->pers = NULL.

If Thread B concurrently writes "inactive" to array_state, it skips setting
MD_CLOSING because mddev->pers is NULL:

drivers/md/md.c:array_state_store() {
...
	case inactive:
...
		if (!mddev->pers || !md_is_rdwr(mddev))
			break;
		err = mddev_set_closing_and_sync_blockdev(mddev, 0);
...
}

Thread B then calls mddev_lock(), which blocks because Thread A holds the
reconfig_mutex. If Thread B receives a signal, mddev_lock() returns -EINTR,
and we jump to out_clear_closing:

>  
>  	switch (st) {
>  	case inactive:

[ ... ]

> @@ -4769,6 +4769,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
>  	}
>  	mddev_unlock(mddev);
>  
> +out_clear_closing:
>  	if (st == readonly || st == read_auto || st == inactive ||
>  	    (err && st == clear))
>  		clear_bit(MD_CLOSING, &mddev->flags);

Because st == inactive, Thread B blindly clears MD_CLOSING, even though it
never set it.

Could this allow a third thread to successfully open the device (since
md_open() only checks MD_CLOSING) while Thread A is actively destroying the
array data structures, leading to a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717041746.363740-1-jinpu.wang@ionos.com?part=1