Re: [PATCH] md/raid1: don't set array_frozen in raid1_takeover()

[email protected] Mon, 03 Aug 2026 18:20:18 +0000
Newsgroups gmane.linux.raid
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:

Pre-existing issues:
- [High] level_store() completely ignores the return value of pers->run(mdd=
ev), leading to a memory leak and leaving the md array in a broken state if=
 the personality initialization fails.
--

--- Patch [1]: [PATCH] md/raid1: don't set array_frozen in raid1_takeover()=
 ---
commit 6300b6117ffff45fb0af7f09f8050ab659c74d0c
Author: Bruce Johnston <[email protected]>

md/raid1: don't set array_frozen in raid1_takeover()

raid1_takeover() sets conf->array_frozen =3D 1 on the newly-allocated
r1conf but since commit b39f35ebe86d removed the quiesce call, it
is never cleared and stalls I/O permanently. This commit fixes the
issue by removing the array_frozen assignment in setup_conf().

Link: https://issues.redhat.com/browse/RHEL-191802

> This used to be harmless: level_store() called mddev_resume() right
> after pers->run(), which called raid1_quiesce(mddev, 0) and cleared

[Severity: High]
This is a pre-existing issue, but looking at level_store(), the code
ignores the return value of pers->run(mddev):

drivers/md/md.c:level_store() {
        ...
        pers->run(mddev);
        set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
        ...
}

If pers->run() fails (for example, if raid1_run() fails during
raid1_set_limits() and expects the caller to free mddev->private),
does this leak the personality-specific configuration structure
(like r1conf and its mempools)?

Since the return value is ignored, could this also leave the array in a
broken state where subsequent I/O causes a crash because the active
personality is not fully initialized?

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c2..2a247ed5fff11 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -3485,8 +3485,6 @@ static void *raid1_takeover(struct mddev *mddev)
>  		mddev->new_chunk_sectors =3D 0;
>  		conf =3D setup_conf(mddev);
>  		if (!IS_ERR(conf)) {
> -			/* Array must appear to be quiesced */
> -			conf->array_frozen =3D 1;
>  			mddev_clear_unsupported_flags(mddev,
>  				UNSUPPORTED_MDDEV_FLAGS);
>  		}

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803180240.1177=
[email protected]?part=3D1