Re: [PATCH] md/raid1: prevent a race between write and stop request

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Critical] Using an interruptible mutex in the I/O path causes normal user-space signals to spuriously fail legitimate writes with I/O errors.
- [High] Global lock acquisition in the high-frequency block I/O fast path completely serializes all RAID1 write requests.
- [High] Acquiring `reconfig_mutex` in the block I/O submission path causes a fatal lock-inversion deadlock during memory reclaim.
- [High] Holding `reconfig_mutex` while sleeping in `wait_blocked_rdev()` indefinitely blocks the MD management thread from clearing the blocked state.

Pre-existing issues:
- [High] The patch only protects WRITE requests, leaving the READ path entirely unpatched and vulnerable to the original race condition causing a NULL pointer dereference.
--

commit 75f3c8447cce610e8b9e2c583ca2b414877078df
Author: Edward Adam Davis [off-list ref]

md/raid1: prevent a race between write and stop request

This commit attempts to fix a race condition between write and stop requests
that leads to a null pointer dereference. It introduces synchronization in
the write path using the mddev lock to ensure the consistent state of mddev
members during a write request.

Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f

[Severity: High]
This is a pre-existing issue, but it appears the read path is still
unprotected and vulnerable to the same race condition.

Since the patch only wraps the write path, if a concurrent do_md_stop() runs
while a READ request is processed, raid1_read_request() will fetch
mddev->private without protection. Could this result in the exact same null
pointer dereference crash on reads when wait_read_barrier() is called?

 	else {
+		int err;
+
 		md_write_start(mddev, bio);
-		if (!raid1_write_request(mddev, bio, sectors))
+		err = mddev_lock(mddev);

[Severity: High]
By acquiring mddev_lock() (which takes the global reconfig_mutex) in the
hot I/O submission path, are all RAID1 write requests completely serialized?
Taking a global reconfiguration mutex on every single write request could
degrade array scalability and I/O concurrency.

[Severity: High]
Could acquiring reconfig_mutex in the block I/O submission path also cause
a lock-inversion deadlock during memory reclaim?

If a thread holds reconfig_mutex (for example, during md_import_device())
and performs a GFP_KERNEL allocation, direct reclaim could write dirty pages
to the RAID array. Since raid1_make_request() attempts to acquire
reconfig_mutex via mddev_lock(), this might cause a direct self-deadlock.

+
+		if (err < 0) {
+			md_write_end(mddev);
+			status = BLK_STS_IOERR;
+			goto done;
+		}

[Severity: Critical]
By translating a failed interruptible mutex lock into BLK_STS_IOERR, could
normal user-space signals spuriously fail legitimate writes?

mddev_lock() uses mutex_lock_interruptible(), which returns -EINTR if a
signal is pending. If a userspace process performing I/O receives a benign
signal (like SIGALRM) while submitting a write, this path seems to fail the
write with an I/O error instead of transparently retrying or blocking.

+
+		if (!mddev->private) {
+			mddev_unlock(mddev);
+			md_write_end(mddev);
+			status = BLK_STS_OFFLINE;
+			goto done;
+		}
+
+		err = raid1_write_request(mddev, bio, sectors);
+		mddev_unlock(mddev);

[Severity: High]
Since reconfig_mutex is held across the entirety of raid1_write_request(),
could this block the MD management thread indefinitely?

If a write request encounters a blocked device and sleeps in
wait_blocked_rdev() inside raid1_write_request(), the I/O thread will be
holding reconfig_mutex. The MD management thread (md_check_recovery()) needs
to clear the blocked state, but it uses mddev_trylock() to acquire
reconfig_mutex. Since the I/O thread holds the lock, the management thread
will fail to acquire it, potentially leaving the I/O thread blocked forever.

+
+		if (!err)
 			md_write_end(mddev);
 	}
+out:
 	return true;
+done:
+	bio->bi_status = status;
+	bio_endio(bio);
+	goto out;
 }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828103957.245658-1-eadavis@sina.com?part=1