* Re: [PATCH v2] md/raid5: reject raid4/5 arrays with too few disks
2026-09-05 10:43 [PATCH v2] md/raid5: reject raid4/5 arrays with too few disks Chen Cheng
@ 2026-09-05 11:00 ` sashiko-bot
2026-09-07 6:35 ` yu kuai
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-09-05 11:00 UTC (permalink / raw)
To: Chen Cheng; +Cc: linux-raid, yukuai
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
^ permalink raw reply [flat|nested] 3+ messages in thread