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

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

[Bruce Johnston]

raid1_takeover() sets conf->array_frozen = 1 on the newly-allocated
r1conf and nothing ever clears it, so every I/O to the array stalls
permanently once _wait_barrier() sees it stuck at 1.

This used to be harmless: level_store() called mddev_resume() right
after pers->run(), which called raid1_quiesce(mddev, 0) and cleared
array_frozen back to 0 regardless of what raid1_takeover() set. Commit
b39f35ebe86d ("md: don't quiesce in mddev_suspend()") removed that
quiesce(mddev, 0) call, so the pre-set now sticks.

setup_conf() already zero-initializes the new r1conf via kzalloc, so
just don't set array_frozen here.

Same class of bug as commit 892da88d1cd9 ("md/raid10: fix a
'conf->barrier' leakage in raid10_takeover()"), also triggered by
b39f35ebe86d.

Fixes: b39f35ebe86d ("md: don't quiesce in mddev_suspend()")
Link: https://issues.redhat.com/browse/RHEL-191802
Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
---
 drivers/md/raid1.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96a..2a247ed5f 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);
 		}
-- 
2.54.0

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

[sashiko-bot]

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

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

[yu kuai]

在 2026/8/4 2:02, Bruce Johnston 写道:

> raid1_takeover() sets conf->array_frozen = 1 on the newly-allocated
> r1conf and nothing ever clears it, so every I/O to the array stalls
> permanently once _wait_barrier() sees it stuck at 1.
>
> This used to be harmless: level_store() called mddev_resume() right
> after pers->run(), which called raid1_quiesce(mddev, 0) and cleared
> array_frozen back to 0 regardless of what raid1_takeover() set. Commit
> b39f35ebe86d ("md: don't quiesce in mddev_suspend()") removed that
> quiesce(mddev, 0) call, so the pre-set now sticks.
>
> setup_conf() already zero-initializes the new r1conf via kzalloc, so
> just don't set array_frozen here.
>
> Same class of bug as commit 892da88d1cd9 ("md/raid10: fix a
> 'conf->barrier' leakage in raid10_takeover()"), also triggered by
> b39f35ebe86d.
>
> Fixes: b39f35ebe86d ("md: don't quiesce in mddev_suspend()")
> Link:https://issues.redhat.com/browse/RHEL-191802
> Signed-off-by: Bruce Johnston<bjohnsto@redhat.com>
> ---
>   drivers/md/raid1.c | 2 --
>   1 file changed, 2 deletions(-)
Applied to md-7.3

-- 
Thanks,
Kuai