Linux RAID subsystem development
 help / color / mirror / Atom feed
From: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
To: song@kernel.org, yukuai@fygo.io, magiclinan@didiglobal.com,
	xiao@kernel.org
Cc: linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org,
	Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>,
	sashiko-bot <sashiko-bot@kernel.org>
Subject: [PATCH] md: avoid modifying spares while the array is not suspended
Date: Tue, 30 Jun 2026 07:56:40 +0000	[thread overview]
Message-ID: <20260630075640.1081634-1-abd.masalkhi@gmail.com> (raw)

remove_spares() and remove_and_add_spares() modify the array's rdev
configuration. These operations are only safe after the array has been
suspended.

Today, md_start_sync() can call md_choose_sync_action() even when the
array has not been suspended. As a result, md_choose_sync_action() may
remove or replace rdevs while normal I/O is still accessing them.

The race can occur as follows:

raid10d          Worker                      Normal IO
____________     _______________________     ______________________

                                             raid10_write_request()
                                             wait_blocked_dev()
set Blocked
set Faulty
                                             Skip Faulty rdev
                                             rrdev->nr_pending++
                                             .repl_bio = bio
                 removeable_rdev = false     .
                 array not suspended         .
lock mddev                                   goto err_handle
                 lock mddev (wait)
                 .
update sb        .
clear Blocked    .
                 .
unlock mddev     .
                 lock mddev (acquires)
                 remove_spares()
                 removeable_rdev = true

                 raid10_remove_disk()
                 rdev = replacement
                 replacement = NULL
                                             rdev_dec_pending(NULL)
                 unlock mddev                (NULL)->nr_pending--

In this case, rdev_dec_pending() is called with a NULL pointer,
resulting in a NULL pointer dereference when attempting to decrement
nr_pending.

Fix this by ensuring that remove_spares() and remove_and_add_spares()
are only called after the array has been suspended, preventing
concurrent rdev configuration changes while normal I/O is in progress.

Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/md.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 66a41d482e59..c85ebb59535b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
 	return spares;
 }
 
-static bool md_choose_sync_action(struct mddev *mddev, int *spares)
+static bool md_choose_sync_action(struct mddev *mddev, int *spares,
+				  bool array_suspended)
 {
 	/* Check if reshape is in progress first. */
 	if (mddev->reshape_position != MaxSector) {
@@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
 
 	/* Check if resync is in progress. */
 	if (mddev->resync_offset < MaxSector) {
-		remove_spares(mddev, NULL);
+		if (array_suspended)
+			remove_spares(mddev, NULL);
+
 		set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 		clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
 		clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
@@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
 	 * also removed and re-added, to allow the personality to fail the
 	 * re-add.
 	 */
-	*spares = remove_and_add_spares(mddev, NULL);
+	if (array_suspended)
+		*spares = remove_and_add_spares(mddev, NULL);
+	else
+		*spares = 0;
+
 	if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
 		clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 		clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
@@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
 		 * As we only add devices that are already in-sync, we can
 		 * activate the spares immediately.
 		 */
-		remove_and_add_spares(mddev, NULL);
+		if (suspend)
+			remove_and_add_spares(mddev, NULL);
 		goto not_running;
 	}
 
-	if (!md_choose_sync_action(mddev, &spares))
+	if (!md_choose_sync_action(mddev, &spares, suspend))
 		goto not_running;
 
 	if (!mddev->pers->sync_request)
-- 
2.43.0


             reply	other threads:[~2026-06-30  7:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30  7:56 Abd-Alrhman Masalkhi [this message]
2026-06-30 10:18 ` [PATCH] md: avoid modifying spares while the array is not suspended sashiko-bot
2026-07-05 16:16 ` yu kuai
2026-07-05 19:58   ` Abd-Alrhman Masalkhi
2026-07-07  1:12     ` yu kuai
2026-07-07 10:35       ` Abd-Alrhman Masalkhi
2026-07-08  8:32         ` yu kuai
2026-07-08  8:59           ` Abd-Alrhman Masalkhi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260630075640.1081634-1-abd.masalkhi@gmail.com \
    --to=abd.masalkhi@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=magiclinan@didiglobal.com \
    --cc=sashiko-bot@kernel.org \
    --cc=song@kernel.org \
    --cc=xiao@kernel.org \
    --cc=yukuai@fygo.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox