[PATCH v2] md: recheck spare changes before starting sync

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

md_start_sync() checks whether spare configuration changes are needed
before taking reconfig_mutex. However, the rdev state can change before
the mutex is acquired, so the initial check can become stale. In that
case, 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 suspending the array when spare configuration changes are
needed, including for non-read-write arrays, and checking again after
taking reconfig_mutex. If the array was not already suspended and a
change is now needed, release the mutex, suspend the array, and
reacquire the mutex before continuing.

Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
Reported-by: sashiko-bot <[email protected]>
Closes: https://sashiko.dev/#/patchset/[email protected]?part=3
Signed-off-by: Abd-Alrhman Masalkhi <[email protected]>
---
Changes in v2:
 - Recheck whether spare configuration changes are needed after taking
   reconfig_mutex, and suspend the array before continuing if necessary.
 - Account for read only arrays, where reshape_position may not be
   MaxSector even though md_start_sync() can still modify the spare
   configuration.
 - Link to v1: https://lore.kernel.org/linux-raid/[email protected]/
---
 drivers/md/md.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 66a41d482e59..c3fc47a82415 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10174,13 +10174,25 @@ static void md_start_sync(struct work_struct *ws)
 	 * If reshape is still in progress, spares won't be added or removed
 	 * from conf until reshape is done.
 	 */
-	if (mddev->reshape_position == MaxSector &&
+	if ((mddev->reshape_position == MaxSector || !md_is_rdwr(mddev)) &&
 	    md_spares_need_change(mddev)) {
 		suspend = true;
 		mddev_suspend(mddev, false);
 	}
 
 	mddev_lock_nointr(mddev);
+
+	/*
+	 * The spare configuration can change before reconfig_mutex is acquired.
+	 * Recheck while holding the lock and suspend if needed.
+	 */
+	if (!suspend && (mddev->reshape_position == MaxSector || !md_is_rdwr(mddev)) &&
+	    md_spares_need_change(mddev)) {
+		mddev_unlock(mddev);
+		mddev_suspend_and_lock_nointr(mddev);
+		suspend = true;
+	}
+
 	if (!md_is_rdwr(mddev)) {
 		/*
 		 * On a read-only array we can:
-- 
2.43.0