[PATCH v2] md/raid5: reject raid4/5 arrays with too few disks

[PATCH v2] md/raid5: reject raid4/5 arrays with too few disks

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

raid4 requires at least two disks and raid5 requires at least three disks,
but setup_conf() only rejects raid6 arrays with fewer than four disks. As a
result, an invalid raid4 or raid5 array can be assembled with no data disks
and later reach raid5_set_limits().

Reject such arrays before the r5conf is created. This prevents the invalid
geometry from reaching queue limit setup, where roundup_pow_of_two() can be
called with a zero stripe size.

Reported-by: syzbot+de94ddbfff0c9e6fe030@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/6a778c82.01d0871a.3a0d52.006a.GAE@google.com/
Fixes: f63f17350e53 ("md/raid5: use the atomic queue limit update APIs")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
v1->v2:
- Fix wrong tags
- Check raid-4 also
---
 drivers/md/raid5.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b91545ce090d..4d8bb0cf2605 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7655,13 +7655,16 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	     && !algorithm_valid_raid6(mddev->new_layout))) {
 		pr_warn("md/raid:%s: layout %d not supported\n",
 			mdname(mddev), mddev->new_layout);
 		return ERR_PTR(-EIO);
 	}
-	if (mddev->new_level == 6 && mddev->raid_disks < 4) {
-		pr_warn("md/raid:%s: not enough configured devices (%d, minimum 4)\n",
-			mdname(mddev), mddev->raid_disks);
+	if ((mddev->new_level == 4 && mddev->raid_disks < 2) ||
+	    (mddev->new_level == 5 && mddev->raid_disks < 3) ||
+	    (mddev->new_level == 6 && mddev->raid_disks < 4)) {
+		pr_warn("md/raid:%s: not enough configured devices (%d, minimum %d)\n",
+			mdname(mddev), mddev->raid_disks,
+			mddev->new_level - 2);
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (!mddev->new_chunk_sectors ||
 	    (mddev->new_chunk_sectors << 9) % PAGE_SIZE ||
-- 
2.55.0

Re: [PATCH v2] md/raid5: reject raid4/5 arrays with too few disks

[sashiko-bot]

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

New issues:
- [High] Rejecting RAID5 arrays with fewer than 3 disks breaks explicitly supported 2-disk RAID5 arrays, preventing assembly and RAID1-to-RAID5 reshaping.

Pre-existing issues:
- [Critical] The patch fails to validate `previous_raid_disks` during a reshape, allowing the syzbot panic (`data_disks == 0`) to still be triggered.
- [High] Null pointer dereference during hotplug teardown in raid5_free_percpu() error path.
- [Medium] Memory leak of disk extra_page allocations on the setup_conf() error path.
--

commit 354e4797772beea1449d8c8f033639134da1c7b9
Author: Chen Cheng <chencheng@fnnas.com>

md/raid5: reject raid4/5 arrays with too few disks

This commit rejects RAID4 arrays with fewer than 2 disks and RAID5 arrays
with fewer than 3 disks during setup_conf(). This is intended to prevent
invalid geometries from reaching queue limit setup and causing a panic
due to a zero stripe size.

Closes: https://lore.kernel.org/r/6a778c82.01d0871a.3a0d52.006a.GAE@google.com/

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090de..4d8bb0cf26055 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7657,9 +7657,12 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  			mdname(mddev), mddev->new_layout);
>  		return ERR_PTR(-EIO);
>  	}
> -	if (mddev->new_level == 6 && mddev->raid_disks < 4) {
> -		pr_warn("md/raid:%s: not enough configured devices (%d, minimum 4)\n",
> -			mdname(mddev), mddev->raid_disks);
> +	if ((mddev->new_level == 4 && mddev->raid_disks < 2) ||
> +	    (mddev->new_level == 5 && mddev->raid_disks < 3) ||

[Severity: High]
Does this unintentionally break 2-disk RAID5 arrays?

The md driver supports and intentionally creates 2-disk RAID5 arrays during a
RAID1-to-RAID5 reshape. For example, in drivers/md/raid5.c
raid5_takeover_raid1() creates a 2-disk RAID5:

    mddev->new_level = 5;
    mddev->new_layout = ALGORITHM_LEFT_SYMMETRIC;
    mddev->new_chunk_sectors = chunksect;

    ret = setup_conf(mddev);

By unconditionally rejecting mddev->new_level == 5 && mddev->raid_disks < 3
here, doesn't this cause valid reshape operations and assembly of legitimate
2-disk RAID5 arrays to fail with -EINVAL?

> +	    (mddev->new_level == 6 && mddev->raid_disks < 4)) {
> +		pr_warn("md/raid:%s: not enough configured devices (%d, minimum %d)\n",
> +			mdname(mddev), mddev->raid_disks,
> +			mddev->new_level - 2);
>  		return ERR_PTR(-EINVAL);
>  	}

[Severity: Critical]
This is a pre-existing issue, but does this actually fix the root cause of
the syzbot panic?

Later in setup_conf(), previous_raid_disks is calculated using an unvalidated
delta_disks from the superblock:

    if (mddev->reshape_position == MaxSector)
        conf->previous_raid_disks = mddev->raid_disks;
    else
        conf->previous_raid_disks = mddev->raid_disks - mddev->delta_disks;

An attacker or fuzzed image could bypass the new minimum disk check by
providing a valid raid_disks (e.g., 3 for RAID5) but supplying a positive
delta_disks (e.g., 2), resulting in previous_raid_disks = 1.

Then in raid5_set_limits():

    data_disks = conf->previous_raid_disks - conf->max_degraded;
    stripe = roundup_pow_of_two(data_disks * (mddev->chunk_sectors << 9));

Since max_degraded is 1 for RAID5, data_disks becomes 0 again, and
roundup_pow_of_two(0) will still trigger the exact same division by zero
panic. Should we also be validating delta_disks or previous_raid_disks?

[Severity: Medium]
This is a pre-existing issue, but does the error path out of setup_conf()
leak the disk extra_page allocations?

In setup_conf(), extra pages are allocated using max_disks as the bound:

    for (i = 0; i < max_disks; i++) {
        conf->disks[i].extra_page = alloc_page(GFP_KERNEL);
        if (!conf->disks[i].extra_page)
            goto abort;
    }

If a subsequent initialization step fails, setup_conf() jumps to the abort
label and calls free_conf(). However, free_conf() iterates using
conf->pool_size to free these pages:

    for (i = 0; i < conf->pool_size; i++)
        if (conf->disks[i].extra_page)
            put_page(conf->disks[i].extra_page);

Since conf->pool_size is zero-initialized in setup_conf() and only populated
later in grow_stripes(), doesn't the cleanup loop terminate immediately and
leak all the successfully allocated pages?

[Severity: High]
This is a pre-existing issue, but does the setup_conf() error path trigger a
NULL pointer dereference during hotplug teardown?

Earlier in setup_conf(), raid5_alloc_percpu() is called:

    conf->percpu = alloc_percpu(struct raid5_percpu);
    if (!conf->percpu)
        return -ENOMEM;

    err = cpuhp_state_add_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
    if (!err) {
        ...

If cpuhp_state_add_instance() fails (for example, due to a memory allocation
failure in raid456_cpu_up_prepare()), it returns an error but leaves
conf->percpu allocated and conf->node unhashed.

When setup_conf() handles the error by jumping to abort and calling
free_conf(), it invokes raid5_free_percpu():

    if (!conf->percpu)
        return;
    
    cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);

Because conf->percpu is non-NULL, cpuhp_state_remove_instance() is called.
The CPU hotplug core then calls hlist_del(&conf->node), which dereferences
the NULL pprev pointer of the unhashed list node. Can this crash the system
under memory pressure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905104323.2202902-1-chencheng@fnnas.com?part=1

Re: [PATCH v2] md/raid5: reject raid4/5 arrays with too few disks

[yu kuai]

Hi,

在 2026/9/5 19:00, sashiko-bot@kernel.org 写道:
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index b91545ce090de..4d8bb0cf26055 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -7657,9 +7657,12 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>>   			mdname(mddev), mddev->new_layout);
>>   		return ERR_PTR(-EIO);
>>   	}
>> -	if (mddev->new_level == 6 && mddev->raid_disks < 4) {
>> -		pr_warn("md/raid:%s: not enough configured devices (%d, minimum 4)\n",
>> -			mdname(mddev), mddev->raid_disks);
>> +	if ((mddev->new_level == 4 && mddev->raid_disks < 2) ||
>> +	    (mddev->new_level == 5 && mddev->raid_disks < 3) ||
> [Severity: High]
> Does this unintentionally break 2-disk RAID5 arrays?
>
> The md driver supports and intentionally creates 2-disk RAID5 arrays during a
> RAID1-to-RAID5 reshape. For example, in drivers/md/raid5.c
> raid5_takeover_raid1() creates a 2-disk RAID5:
>
>      mddev->new_level = 5;
>      mddev->new_layout = ALGORITHM_LEFT_SYMMETRIC;
>      mddev->new_chunk_sectors = chunksect;
>
>      ret = setup_conf(mddev);
>
> By unconditionally rejecting mddev->new_level == 5 && mddev->raid_disks < 3
> here, doesn't this cause valid reshape operations and assembly of legitimate
> 2-disk RAID5 arrays to fail with -EINVAL?

This make sense, 2 disks raid1 reshape to raid5 should not be broken.

-- 
Thanks,
Kuai