linux-kernel - [PATCH -next 3/8] md: return error to user if idle/frozen_sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231110172834.3939490-4-yukuai1@huaweicloud.com>
Date:   Sat, 11 Nov 2023 01:28:29 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     song@...nel.org, xni@...hat.com, yukuai3@...wei.com, neilb@...e.de
Cc:     linux-kernel@...r.kernel.org, linux-raid@...r.kernel.org,
        yukuai1@...weicloud.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: [PATCH -next 3/8] md: return error to user if idle/frozen_sync_thread() is interrupted

From: Yu Kuai <yukuai3@...wei.com>

It doesn't make sense that "echo idle/forzon > sync_action" doesn't
return error to user if idle/frozen_sync_thread() is interrupted.

Also make sure array recovery flags is not changed if error is returned.

Fixes: 8e8e2518fcec ("md: Close race when setting 'action' to 'idle'.")
Signed-off-by: Yu Kuai <yukuai3@...wei.com>
Signed-off-by: Yu Kuai <yukuai1@...weicloud.com>
---
 drivers/md/md.c | 70 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 22 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 5c9387369de1..d7b9d597b54d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4848,13 +4848,16 @@ action_show(struct mddev *mddev, char *page)
 	return sprintf(page, "%s\n", type);
 }
 
-static void stop_sync_thread(struct mddev *mddev)
+static int stop_sync_thread(struct mddev *mddev)
 {
+	int ret = 0;
+
 	if (!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
-		return;
+		return 0;
 
-	if (mddev_lock(mddev))
-		return;
+	ret = mddev_lock(mddev);
+	if (ret)
+		return ret;
 
 	/*
 	 * Check again in case MD_RECOVERY_RUNNING is cleared before lock is
@@ -4862,7 +4865,7 @@ static void stop_sync_thread(struct mddev *mddev)
 	 */
 	if (!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
 		mddev_unlock(mddev);
-		return;
+		return 0;
 	}
 
 	if (work_pending(&mddev->sync_work))
@@ -4876,50 +4879,69 @@ static void stop_sync_thread(struct mddev *mddev)
 	md_wakeup_thread_directly(mddev->sync_thread);
 
 	mddev_unlock(mddev);
+	return 0;
 }
 
-static void idle_sync_thread(struct mddev *mddev)
+static int idle_sync_thread(struct mddev *mddev)
 {
+	int ret;
 	int sync_seq = atomic_read(&mddev->sync_seq);
+	bool flag;
 
-	if (mutex_lock_interruptible(&mddev->sync_mutex))
-		return;
+	ret = mutex_lock_interruptible(&mddev->sync_mutex);
+	if (ret)
+		return ret;
 
-	clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
-	stop_sync_thread(mddev);
+	flag = test_and_clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
+	ret = stop_sync_thread(mddev);
+	if (ret)
+		goto out;
 
-	wait_event_interruptible(resync_wait,
+	ret = wait_event_interruptible(resync_wait,
 			sync_seq != atomic_read(&mddev->sync_seq) ||
 			!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery));
-
+out:
+	if (ret && flag)
+		set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
 	mutex_unlock(&mddev->sync_mutex);
+	return ret;
 }
 
-static void frozen_sync_thread(struct mddev *mddev)
+static int frozen_sync_thread(struct mddev *mddev)
 {
-	if (mutex_lock_interruptible(&mddev->sync_mutex))
-		return;
+	int ret = mutex_lock_interruptible(&mddev->sync_mutex);
+	bool flag;
 
-	set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
-	stop_sync_thread(mddev);
+	if (ret)
+		return ret;
 
-	wait_event_interruptible(resync_wait, mddev->sync_thread == NULL &&
-			!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery));
+	flag = test_and_set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
+	ret = stop_sync_thread(mddev);
+	if (ret)
+		goto out;
 
+	ret = wait_event_interruptible(resync_wait,
+			mddev->sync_thread == NULL &&
+			!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery));
+out:
+	if (ret && !flag)
+		clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
 	mutex_unlock(&mddev->sync_mutex);
+	return ret;
 }
 
 static ssize_t
 action_store(struct mddev *mddev, const char *page, size_t len)
 {
+	int ret = 0;
+
 	if (!mddev->pers || !mddev->pers->sync_request)
 		return -EINVAL;
 
-
 	if (cmd_match(page, "idle"))
-		idle_sync_thread(mddev);
+		ret = idle_sync_thread(mddev);
 	else if (cmd_match(page, "frozen"))
-		frozen_sync_thread(mddev);
+		ret = frozen_sync_thread(mddev);
 	else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
 		return -EBUSY;
 	else if (cmd_match(page, "resync"))
@@ -4963,6 +4985,10 @@ action_store(struct mddev *mddev, const char *page, size_t len)
 		set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
 		set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 	}
+
+	if (ret)
+		return ret;
+
 	if (mddev->ro == MD_AUTO_READ) {
 		/* A write to sync_action is enough to justify
 		 * canceling read-auto mode
-- 
2.39.2