[PATCH v4] md: don't leak MD_CLOSING when array_state write is interrupted

[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 to
keep the array from being reopened while it is 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 catches a signal while blocked there, it returns -EINTR and
MD_CLOSING is left set on a still-running array. 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 shows up in /proc/mdstat and sysfs is fully populated, yet cannot
be opened, and cannot be recovered without a reboot (all clear paths
either need the device open or re-enter test_and_set_bit(MD_CLOSING) and
bail with -EBUSY before the clear).

The window is easy to hit on a busy host: reconfig_mutex is held for long
stretches by a running resync/recovery, widening the interruptible wait,
and a userspace helper writing to array_state and then being killed or
timed out is enough to leak the flag.

Clear the flag directly in the interrupted-lock exit. Guard the clear
with a set_closing flag that records whether this write actually set
MD_CLOSING: the set call is skipped when the array is not running
read-write (or mddev->pers is NULL), and in that case this write must
not clear a MD_CLOSING it never set. This also tightens the pre-existing
tail clear, which had the same unconditional behaviour.

Fixes: 99b902ac1725 ("md: sync blockdev before stopping raid or setting readonly")
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
v4: (Kuai) drop the Assisted-by tag, checkpatch does not accept it.
    Keep the set_closing guard at the tail clear but still skip it after a
    successful clear: md_clean() keeps MD_CLOSING set (except for legacy
    async del_gendisk with hold_active), so clearing it here would let the
    stopped array be reopened.
v3: (Kuai) clear MD_CLOSING directly at the mddev_lock() failure and
    return, drop the out_clear_closing label; reword the commit message.
    Keep the set_closing guard: the set call is skipped when the array
    is not rdwr or mddev->pers is NULL, so this write may not own the
    flag. Rebased on v7.4-rc1.
v2: guard the clear with set_closing.

 drivers/md/md.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 03e35386a802..fb9c4d12c4c5 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4671,6 +4671,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 {
 	int err = 0;
 	enum array_state st = match_word(buf, array_states);
+	bool set_closing = false;
 
 	/* No lock dependent actions */
 	switch (st) {
@@ -4691,6 +4692,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 		err = mddev_set_closing_and_sync_blockdev(mddev, 0);
 		if (err)
 			return err;
+		set_closing = true;
 		break;
 	default:
 		break;
@@ -4718,8 +4720,12 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 		return err ?: len;
 	}
 	err = mddev_lock(mddev);
-	if (err)
+	if (err) {
+		/* Interrupted before any teardown ran, don't leak MD_CLOSING */
+		if (set_closing)
+			clear_bit(MD_CLOSING, &mddev->flags);
 		return err;
+	}
 
 	switch (st) {
 	case inactive:
@@ -4792,8 +4798,8 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 	}
 	mddev_unlock(mddev);
 
-	if (st == readonly || st == read_auto || st == inactive ||
-	    (err && st == clear))
+	/* A successful clear must keep MD_CLOSING set, see md_clean() */
+	if (set_closing && (st != clear || err))
 		clear_bit(MD_CLOSING, &mddev->flags);
 
 	return err ?: len;
-- 
2.43.0