[PATCH] md: validate raid_disks range before loading personality

[PATCH] md: validate raid_disks range before loading personality

[Ran Hongyun]

md_set_array_info() accepts raid_disks from user space without any
range check. The value is stored directly into mddev->raid_disks and
flows unchecked into md_run(), which only tests for zero. A negative
or excessively large raid_disks then reaches the personality ->run()
callback, where it scales several allocations:

  raid0:  kvzalloc(array3_size(..., raid_disks)) in create_strip_zones
  raid1:  kzalloc(array3_size(..., raid_disks, 2)) for mirrors and
          mempool_create_kmalloc_pool(offsetof(r1bio, bios[raid_disks*2]))
  raid5:  kzalloc_objs(disk_info, max_disks) and alloc_page() loop
  raid10: kzalloc_objs(raid10_info, raid_disks + ...) for mirrors

cf5963d6781dd ("md/raid0: validate device count before allocating
devlist") already fixes the raid0 warning. However raid1/5/10 still
hit the MAX_PAGE_ORDER WARN, and simply switching their kzalloc_objs
calls to kvzalloc_objs does not help because kvmalloc_node rejects
sizes above INT_MAX. The proper fix is to validate raid_disks before
any allocation takes place.

Add a MAX_RAID_DEVICES (253) bounds check in md_run() before
pers->run. This is the single choke point covering ioctl RUN_ARRAY,
dm-raid ctr, and reshape paths. Move the existing dm-raid private
definition to md.h so both subsystems share the same limit.

Signed-off-by: Ran Hongyun <ranhongyun1@huawei.com>
---
 drivers/md/dm-raid.c | 1 -
 drivers/md/md.c      | 7 +++++++
 drivers/md/md.h      | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index ac019a0f5a92..29c41a04773e 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -18,7 +18,6 @@
 #include <linux/device-mapper.h>
 
 #define DM_MSG_PREFIX "raid"
-#define	MAX_RAID_DEVICES	253 /* md-raid kernel limit */
 
 /*
  * Minimum sectors of free reshape space per raid device
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 03e35386a802..7b202b483e98 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6703,6 +6703,13 @@ int md_run(struct mddev *mddev)
 			return -EINVAL;
 	}
 
+	if (mddev->raid_disks <= 0 ||
+	    mddev->raid_disks > MAX_RAID_DEVICES) {
+		pr_warn("md: %s: raid_disks %d out of range [1, %d]\n",
+			mdname(mddev), mddev->raid_disks, MAX_RAID_DEVICES);
+		return -EINVAL;
+	}
+
 	if (mddev->level != LEVEL_NONE)
 		request_module("md-level-%d", mddev->level);
 	else if (mddev->clevel[0])
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 6440da292105..75d7f4b14d39 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -22,6 +22,9 @@
 #include <trace/events/block.h>
 
 #define MaxSector (~(sector_t)0)
+
+#define MAX_RAID_DEVICES	253 /* md-raid kernel limit */
+
 /*
  * Number of guaranteed raid bios in case of extreme VM load:
  */
-- 
2.52.0

Re: [PATCH] md: validate raid_disks range before loading personality

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] md: md_run() cap at MAX_RAID_DEVICES breaks valid arrays with >
  253 disks
- [Medium] md: unchecked delta_disks in md_run() allows OOM warn and
  divide-by-zero
- [Medium] md: missing raid_disks upper bound in update_raid_disks() on
  online reshape
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] md/raid1: missing raid_disks upper bounds check in
  raid1_reshape() [1]
--

commit fc41ec678e6b01e865c547317a0225da7de795ee
Author: Ran Hongyun <ranhongyun1@huawei.com>

md: validate raid_disks range before loading personality

This commit moves MAX_RAID_DEVICES (253) from dm-raid.c to md.h and rejects
mddev->raid_disks values outside [1, MAX_RAID_DEVICES] in md_run() before
invoking the personality run callback.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 03e35386a8028..7b202b483e98c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6703,6 +6703,13 @@ int md_run(struct mddev *mddev)
>  			return -EINVAL;
>  	}
>  
> +	if (mddev->raid_disks <= 0 ||
> +	    mddev->raid_disks > MAX_RAID_DEVICES) {
> +		pr_warn("md: %s: raid_disks %d out of range [1, %d]\n",
> +			mdname(mddev), mddev->raid_disks, MAX_RAID_DEVICES);
> +		return -EINVAL;
> +	}
> +
>  	if (mddev->level != LEVEL_NONE)
>  		request_module("md-level-%d", mddev->level);
>  	else if (mddev->clevel[0])

[Severity: High]
Does capping mddev->raid_disks at MAX_RAID_DEVICES (253) in md_run() break
assembling valid native MD arrays with more than 253 member devices?

MAX_RAID_DEVICES was a dm-raid target limit, whereas native MD v1.x
superblocks support up to 384 devices in a 1 KiB superblock and up to 1920
devices in a 4 KiB superblock:

drivers/md/md.c:super_1_validate() {
	...
	mddev->max_disks =  (4096-256)/2;
	...
}

In addition, raid6_takeover() converts a 253-disk RAID5 array into a
254-disk RAID6 array by incrementing mddev->raid_disks to 254, which
md_run() will now refuse to assemble with -EINVAL.

[Severity: Medium]
When an array is started with an active reshape (mddev->reshape_position !=
MaxSector), can an unvalidated mddev->delta_disks bypass this check and
still trigger the oversized allocations or a divide-by-zero?

On an inactive array, writing a large value to sysfs raid_disks, setting
reshape_position, and then writing a small valid value to raid_disks (or
loading a superblock with a crafted delta_disks) leaves mddev->raid_disks
within [1, MAX_RAID_DEVICES] while mddev->delta_disks is arbitrarily
negative.  raid5 setup_conf() and raid10 setup_conf() size their disk
allocations from mddev->raid_disks - mddev->delta_disks:

drivers/md/raid10.c:setup_conf() {
	...
	conf->mirrors = kzalloc_objs(struct raid10_info,
				     mddev->raid_disks + max(0, -mddev->delta_disks));
	...
}

This still hits the MAX_PAGE_ORDER warning (and overflows a signed int if
mddev->delta_disks == INT_MIN).  Furthermore, if mddev->delta_disks equals
mddev->raid_disks, conf->prev.raid_disks (mddev->raid_disks -
mddev->delta_disks) becomes 0 and raid10_run() -> enough() -> _enough()
divides by zero at (this + 1) % disks.

Checking that mddev->raid_disks - mddev->delta_disks is also within the
valid range when mddev->reshape_position != MaxSector would close this gap.

[Severity: Medium]
Does online reshape via update_raid_disks() bypass md_run() and leave
raid1, raid5, and raid10 exposed to the same unbounded allocations?

When reshaping an already-running array via sysfs raid_disks or
ioctl(SET_ARRAY_INFO), raid_disks_store() and update_array_info() call
update_raid_disks() directly without going through md_run():

drivers/md/md.c:update_raid_disks() {
	...
	if (raid_disks <= 0 ||
	    (mddev->max_disks && raid_disks >= mddev->max_disks))
		return -EINVAL;
	...
	mddev->delta_disks = raid_disks - mddev->raid_disks;
	...
	rv = mddev->pers->check_reshape(mddev);
	...
}

When mddev->max_disks is 0 (such as on non-persistent or external-metadata
arrays), any positive raid_disks up to INT_MAX is accepted and passed to
pers->check_reshape(), hitting the MAX_PAGE_ORDER and INT_MAX allocation
warnings in raid1_reshape(), raid10_check_reshape(), and raid5
check_reshape().

[Severity: Medium]
This problem wasn't introduced by this patch, but raid1_reshape() [1]
lacks an upper bounds check on raid_disks when reshaping a running array
with mddev->max_disks == 0:

drivers/md/raid1.c:raid1_reshape() {
	...
	raid_disks = mddev->raid_disks + mddev->delta_disks;
	...
	new_r1bio_size = offsetof(struct r1bio, bios[raid_disks * 2]);
	newpool = mempool_create_kmalloc_pool(NR_RAID_BIOS, new_r1bio_size);
	if (!newpool) {
		return -ENOMEM;
	}
	newmirrors = kzalloc(array3_size(sizeof(struct raid1_info),
					 raid_disks, 2),
			     GFP_KERNEL);
	...
}

Because update_raid_disks() does not bound raid_disks when mddev->max_disks
is 0, a large raid_disks value overflows raid_disks * 2 in signed 32-bit
arithmetic or exceeds KMALLOC_MAX_SIZE in mempool_create_kmalloc_pool() and
kzalloc(), triggering a slab warning in ___kmalloc_large_node().

[1] https://sashiko.dev/#/bug/linux-9d642fe4-31fe-4998-ac3d-c07a30507ae7

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109025621.2031722-1-ranhongyun1@huawei.com?part=1