[PATCH] md/raid0: validate device count before allocating devlist

[PATCH] md/raid0: validate device count before allocating devlist

[Chandradhar Kumar]

create_strip_zones() allocates conf->devlist based on mddev->raid_disks
before verifying that enough devices are present. Validate the number of
member devices before allocating devlist to reject invalid
configurations early.

The existing validation later in the function already rejects this
condition, but it occurs after the potentially excessive allocation.

Reported-by: syzbot+a32ff75e417c0f49a8e9@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=a32ff75e417c0f49a8e9
Signed-off-by: Chandradhar Kumar <chandradhar.2003@gmail.com>
---
 drivers/md/raid0.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 35e103f0c2c3..586e3411f24c 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -69,7 +69,7 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	sector_t curr_zone_end, sectors;
 	struct md_rdev *smallest, *rdev1, *rdev2, *rdev, **dev;
 	struct strip_zone *zone;
-	int cnt;
+	int cnt, nr_devs;
 	struct r0conf *conf = kzalloc_obj(*conf);
 	unsigned int blksize = 512;
 
@@ -79,7 +79,10 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	*private_conf = ERR_PTR(-ENOMEM);
 	if (!conf)
 		return -ENOMEM;
+
+	nr_devs = 0;
 	rdev_for_each(rdev1, mddev) {
+		nr_devs++;
 		pr_debug("md/raid0:%s: looking at %pg\n",
 			 mdname(mddev),
 			 rdev1->bdev);
@@ -147,6 +150,14 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	conf->strip_zone = kvzalloc_objs(struct strip_zone, conf->nr_strip_zones);
 	if (!conf->strip_zone)
 		goto abort;
+
+	if (nr_devs < mddev->raid_disks) {
+		pr_warn("md/raid0:%s: too few disks (%d of %d) - aborting!\n",
+			mdname(mddev), nr_devs, mddev->raid_disks);
+		err = -EINVAL;
+		goto abort;
+	}
+
 	conf->devlist = kvzalloc(array3_size(sizeof(struct md_rdev *),
 					     conf->nr_strip_zones,
 					     mddev->raid_disks),
-- 
2.55.0

Re: [PATCH] md/raid0: validate device count before allocating devlist

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added validation `if (nr_devs < mddev->raid_disks)` uses signed integer comparison, which can be bypassed if `mddev->raid_disks` is set to a negative value from user-space, resulting in an incomplete fix.
--

commit b0e2bd0b0a05bf7824ef3d801efa4d7542d36bbc
Author: Chandradhar Kumar <chandradhar.2003@gmail.com>

md/raid0: validate device count before allocating devlist

This commit adds early validation to check if there are enough member
devices before allocating the device list. It aims to prevent excessive
allocations that trigger warnings on invalid configurations.

Link: https://syzbot.org/bug?extid=a32ff75e417c0f49a8e9

> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 35e103f0c2c3e..586e3411f24c1 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
[ ... ]
> @@ -147,6 +150,14 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
>  	conf->strip_zone = kvzalloc_objs(struct strip_zone, conf->nr_strip_zones);
>  	if (!conf->strip_zone)
>  		goto abort;
> +
> +	if (nr_devs < mddev->raid_disks) {
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
Does this signed comparison allow bypassing the early abort if a negative
value is provided from user-space?

If a user issues the SET_ARRAY_INFO ioctl with a negative value for
raid_disks, it gets assigned directly without validation:

drivers/md/md.c:md_set_array_info() {
    ...
    mddev->raid_disks    = info->raid_disks;
    ...
}

When followed by the RUN_ARRAY ioctl, this validation check in
create_strip_zones() evaluates to false (e.g., 1 < -1). This bypasses the
check and allows the negative value to be cast to SIZE_MAX in array3_size()
which is then passed to kvzalloc().

Could this trigger the exact same warning for huge memory allocations that
this patch was explicitly intended to fix?

> +		pr_warn("md/raid0:%s: too few disks (%d of %d) - aborting!\n",
> +			mdname(mddev), nr_devs, mddev->raid_disks);
> +		err = -EINVAL;
> +		goto abort;
> +	}
> +
>  	conf->devlist = kvzalloc(array3_size(sizeof(struct md_rdev *),
>  					     conf->nr_strip_zones,
>  					     mddev->raid_disks),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906143928.105165-1-chandradhar.2003@gmail.com?part=1

[PATCH v2] md/raid0: validate device count before allocating devlist

[Chandradhar Kumar]

create_strip_zones() allocates conf->devlist based on mddev->raid_disks
before verifying that enough devices are present. Validate the number of
member devices before allocating devlist to reject invalid
configurations early.

The existing validation later in the function already rejects this
condition, but it occurs after the potentially excessive allocation.

Reported-by: syzbot+a32ff75e417c0f49a8e9@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=a32ff75e417c0f49a8e9
Signed-off-by: Chandradhar Kumar <chandradhar.2003@gmail.com>

v2:
- Reject non-positive raid-disks values, which can bypass the device
  count validation and result in an oversized allocation.
---
 drivers/md/raid0.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 35e103f0c2c3..8eaf078606d6 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -69,7 +69,7 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	sector_t curr_zone_end, sectors;
 	struct md_rdev *smallest, *rdev1, *rdev2, *rdev, **dev;
 	struct strip_zone *zone;
-	int cnt;
+	int cnt, nr_devs;
 	struct r0conf *conf = kzalloc_obj(*conf);
 	unsigned int blksize = 512;
 
@@ -79,7 +79,10 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	*private_conf = ERR_PTR(-ENOMEM);
 	if (!conf)
 		return -ENOMEM;
+
+	nr_devs = 0;
 	rdev_for_each(rdev1, mddev) {
+		nr_devs++;
 		pr_debug("md/raid0:%s: looking at %pg\n",
 			 mdname(mddev),
 			 rdev1->bdev);
@@ -144,6 +147,21 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 	}
 
 	err = -ENOMEM;
+
+	if (mddev->raid_disks <= 0) {
+		pr_warn("md/raid0:%s: invalid number of disks %d - aborting!\n",
+			mdname(mddev), mddev->raid_disks);
+		err = -EINVAL;
+		goto abort;
+	}
+
+	if (nr_devs < mddev->raid_disks) {
+		pr_warn("md/raid0:%s: too few disks (%d of %d) - aborting!\n",
+			mdname(mddev), nr_devs, mddev->raid_disks);
+		err = -EINVAL;
+		goto abort;
+	}
+
 	conf->strip_zone = kvzalloc_objs(struct strip_zone, conf->nr_strip_zones);
 	if (!conf->strip_zone)
 		goto abort;
-- 
2.55.0