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

[sashiko-bot@kernel.org]

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(mddev), 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 <bjohnsto@redhat.com>

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

raid1_takeover() sets conf->array_frozen = 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 = 0;
>  		conf = setup_conf(mddev);
>  		if (!IS_ERR(conf)) {
> -			/* Array must appear to be quiesced */
> -			conf->array_frozen = 1;
>  			mddev_clear_unsupported_flags(mddev,
>  				UNSUPPORTED_MDDEV_FLAGS);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180240.1177104-1-bjohnsto@redhat.com?part=1