Re: [PATCH v2] md: scope memalloc_noio to allocation critical sections

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

Pre-existing issues:
- [High] In `backlog_store()`, an error path unlocks the mddev but fails to resume it, leaving the array permanently suspended.
--

--- Patch [1]: [PATCH v2] md: scope memalloc_noio to allocation critical sections ---
commit 6df6cebcdd6ace34da5732a820e0ad95df14d0fc
Author: Chen Cheng <[email protected]>

    md: scope memalloc_noio to allocation critical sections

This patch fixes a token leak issue where PF_MEMALLOC_NOIO could leak across
tasks when saving the token in mddev_suspend() and restoring it in
mddev_resume(). It refactors the code to call memalloc_noio_save() directly
before memory allocation in mdraid operations after the array has been
successfully suspended.

Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh

> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 0f02e2956398d..f005bc9494ecf 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2629,10 +2629,12 @@ static ssize_t
>  location_store(struct mddev *mddev, const char *buf, size_t len)
>  {
>  	int rv;
> +	unsigned int noio_flags;
>  
>  	rv = mddev_suspend_and_lock(mddev);
>  	if (rv)
>  		return rv;
> +	noio_flags = memalloc_noio_save();

[ ... ]

While reviewing this file, I noticed a pre-existing issue in backlog_store():

drivers/md/md-bitmap.c:backlog_store() {
    ...
	rv = mddev_suspend_and_lock(mddev);
	if (rv)
		return rv;

	/*
	 * Without write mostly device, it doesn't make sense to set
	 * backlog for max_write_behind.
	 */
	rdev_for_each(rdev, mddev) {
		if (test_bit(WriteMostly, &rdev->flags)) {
			has_write_mostly = true;
			break;
		}
	}
	if (!has_write_mostly) {
		pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
				    mdname(mddev));
		mddev_unlock(mddev);
		return -EINVAL;
	}
    ...
}

[Severity: High]
This is a pre-existing issue, but the error path for !has_write_mostly in
backlog_store() calls mddev_unlock(mddev) instead of
mddev_unlock_and_resume(mddev).

If a privileged user writes to the bitmap/backlog sysfs file when there are no
write-mostly devices in the array, mddev_suspend_and_lock(mddev) is called, but
the array is never resumed on this error path. 

Does this permanently leave mddev->suspended elevated, blocking normal I/O
processing and any future suspend/resume operations on the array?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1