[PATCH] md: fix out-of-bounds access to superblock disk array

[PATCH] md: fix out-of-bounds access to superblock disk array

[Deepanshu Kartikey]

The v0.90 superblock format can only describe MD_SB_DISKS (27) member
disks, but several places did not enforce this limit, allowing an
out-of-bounds access to sb->disks[]:

 1. super_90_load() only checked that sb->raid_disks was positive,
    not that it fit within MD_SB_DISKS. A crafted superblock with an
    oversized raid_disks value was accepted into mddev->raid_disks.

 2. super_90_sync() computed desc_nr from rdev2->raid_disk or a
    running spare counter without checking it stayed within bounds
    before indexing sb->disks[desc_nr].

 3. super_90_sync() also indexed sb->disks[rdev->desc_nr] for
    sb->this_disk without any bounds check.

 4. The "missing devices" loop in super_90_sync() iterated up to
    mddev->raid_disks with no per-iteration bound on sb->disks[].

Any of these could be reached with a bad raid_disks/desc_nr value
and cause an out-of-bounds array access:

  UBSAN: array-index-out-of-bounds in drivers/md/md.c:1697:17
  index 124 is out of range for type 'mdp_disk_t [27]'

Validate raid_disks at load time, and add bounds checks at each
point sb->disks[] is indexed as defense in depth.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+9e3014263a35700ab49b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9e3014263a35700ab49b
Assisted-by: Claude
Tested-by: syzbot+9e3014263a35700ab49b@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/md/md.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..f950e6fd144c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1393,7 +1393,8 @@ static int super_90_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor
 		goto abort;
 	}
 
-	if (sb->raid_disks <= 0)
+	if (sb->raid_disks <= 0 ||
+	    sb->raid_disks > MD_SB_DISKS)
 		goto abort;
 
 	if (md_csum_fold(calc_sb_csum(sb)) != md_csum_fold(sb->sb_csum)) {
@@ -1693,6 +1694,13 @@ static void super_90_sync(struct mddev *mddev, struct md_rdev *rdev)
 			desc_nr = rdev2->raid_disk;
 		else
 			desc_nr = next_spare++;
+
+		if (desc_nr < 0 || desc_nr >= MD_SB_DISKS) {
+			pr_warn("md: %s: desc_nr %d out of range for rdev %pg, skipping\n",
+				mdname(mddev), desc_nr, rdev2->bdev);
+			continue;
+		}
+
 		rdev2->desc_nr = desc_nr;
 		d = &sb->disks[rdev2->desc_nr];
 		nr_disks++;
@@ -1722,7 +1730,7 @@ static void super_90_sync(struct mddev *mddev, struct md_rdev *rdev)
 			d->state |= (1<<MD_DISK_FAILFAST);
 	}
 	/* now set the "removed" and "faulty" bits on any missing devices */
-	for (i=0 ; i < mddev->raid_disks ; i++) {
+	for (i = 0 ; i < mddev->raid_disks && i < MD_SB_DISKS ; i++) {
 		mdp_disk_t *d = &sb->disks[i];
 		if (d->state == 0 && d->number == 0) {
 			d->number = i;
@@ -1737,8 +1745,11 @@ static void super_90_sync(struct mddev *mddev, struct md_rdev *rdev)
 	sb->working_disks = working;
 	sb->failed_disks = failed;
 	sb->spare_disks = spare;
-
-	sb->this_disk = sb->disks[rdev->desc_nr];
+	if (rdev->desc_nr >= 0 && rdev->desc_nr < MD_SB_DISKS)
+		sb->this_disk = sb->disks[rdev->desc_nr];
+	else
+		pr_warn("md: %s: rdev desc_nr %d out of range, this_disk not set\n",
+			mdname(mddev), rdev->desc_nr);
 	sb->sb_csum = calc_sb_csum(sb);
 }
 
-- 
2.34.1

Re: [PATCH] md: fix out-of-bounds access to superblock disk array

[sashiko-bot]

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

New issues:
- [High] Skipping devices with an out-of-bounds `desc_nr` in `super_90_sync()` leaves `rdev2->desc_nr` stale, silently corrupting the superblock's `this_disk` field.

Pre-existing issues:
- [High] The patch fails to validate `sb->this_disk.number` in `super_90_load()`, allowing an unvalidated value to be assigned to `rdev->desc_nr`, which subsequently leads to out-of-bounds array reads in `super_90_validate()`.
--

commit b64c4503567571cb6b3f8a25617dfc053378beea
Author: Deepanshu Kartikey <kartikey406@gmail.com>
md: fix out-of-bounds access to superblock disk array

This commit adds bounds checks for raid_disks and desc_nr when loading and
syncing v0.90 superblocks. It aims to prevent out-of-bounds array accesses
to the 27-element sb->disks array caused by maliciously crafted or
corrupted metadata.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..81588c3fee49a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1393,7 +1393,8 @@ static int super_90_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor
>  		goto abort;
>  	}
>  
> -	if (sb->raid_disks <= 0)
> +	if (sb->raid_disks <= 0 ||
> +	    sb->raid_disks > MD_SB_DISKS)
>  		goto abort;
>  
>  	if (md_csum_fold(calc_sb_csum(sb)) != md_csum_fold(sb->sb_csum)) {

[Severity: High]
This is a pre-existing issue, but while we are adding bounds checks to
super_90_load(), does this function still leave a critical path vulnerable?

Slightly further down in super_90_load(), the code does:

    rdev->desc_nr = sb->this_disk.number;

Since sb->this_disk.number is read from the untrusted superblock without
validation against MD_SB_DISKS, an unvalidated value is assigned directly
to rdev->desc_nr. Later during array assembly, super_90_validate() 
unconditionally uses rdev->desc_nr to index the 27-element sb->disks array:

    desc = sb->disks + rdev->desc_nr;
    if (desc->state & (1<<MD_DISK_FAULTY))

Can a maliciously crafted MD superblock still trigger an out-of-bounds
memory read through this unvalidated rdev->desc_nr?

[ ... ]

> @@ -1693,6 +1694,13 @@ static void super_90_sync(struct mddev *mddev, struct md_rdev *rdev)
>  			desc_nr = rdev2->raid_disk;
>  		else
>  			desc_nr = next_spare++;
> +
> +		if (desc_nr < 0 || desc_nr >= MD_SB_DISKS) {
> +			pr_warn("md: %s: desc_nr %d out of range for rdev %pg, skipping\n",
> +				mdname(mddev), desc_nr, rdev2->bdev);
> +			continue;
> +		}
> +

[Severity: High]
Does skipping out-of-bounds devices here leave rdev2->desc_nr with a stale
value?

By using continue to early-exit the loop, we skip the subsequent
rdev2->desc_nr = desc_nr assignment. Later in this same function, the
superblock's this_disk field is populated using rdev->desc_nr:

    if (rdev->desc_nr >= 0 && rdev->desc_nr < MD_SB_DISKS)
        sb->this_disk = sb->disks[rdev->desc_nr];

If the target device was skipped during the rdev_for_each loop, will it
reuse its stale desc_nr and copy the descriptor of whichever other active
device took that slot, potentially causing metadata corruption?

>  		rdev2->desc_nr = desc_nr;
>  		d = &sb->disks[rdev2->desc_nr];
>  		nr_disks++;

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

Re: [PATCH] md: fix out-of-bounds access to superblock disk array

[yu kuai]

Hi,

在 2026/9/14 14:06, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Skipping devices with an out-of-bounds `desc_nr` in `super_90_sync()` leaves `rdev2->desc_nr` stale, silently corrupting the superblock's `this_disk` field.
>
> Pre-existing issues:
> - [High] The patch fails to validate `sb->this_disk.number` in `super_90_load()`, allowing an unvalidated value to be assigned to `rdev->desc_nr`, which subsequently leads to out-of-bounds array reads in `super_90_validate()`.
> --
>
> commit b64c4503567571cb6b3f8a25617dfc053378beea
> Author: Deepanshu Kartikey <kartikey406@gmail.com>
> md: fix out-of-bounds access to superblock disk array
>
> This commit adds bounds checks for raid_disks and desc_nr when loading and
> syncing v0.90 superblocks. It aims to prevent out-of-bounds array accesses
> to the 27-element sb->disks array caused by maliciously crafted or
> corrupted metadata.
>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 680b34a63cb3d..81588c3fee49a 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -1393,7 +1393,8 @@ static int super_90_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor
>>   		goto abort;
>>   	}
>>   
>> -	if (sb->raid_disks <= 0)
>> +	if (sb->raid_disks <= 0 ||
>> +	    sb->raid_disks > MD_SB_DISKS)
>>   		goto abort;
>>   
>>   	if (md_csum_fold(calc_sb_csum(sb)) != md_csum_fold(sb->sb_csum)) {
> [Severity: High]
> This is a pre-existing issue, but while we are adding bounds checks to
> super_90_load(), does this function still leave a critical path vulnerable?
>
> Slightly further down in super_90_load(), the code does:
>
>      rdev->desc_nr = sb->this_disk.number;
>
> Since sb->this_disk.number is read from the untrusted superblock without
> validation against MD_SB_DISKS, an unvalidated value is assigned directly
> to rdev->desc_nr. Later during array assembly, super_90_validate()
> unconditionally uses rdev->desc_nr to index the 27-element sb->disks array:
>
>      desc = sb->disks + rdev->desc_nr;
>      if (desc->state & (1<<MD_DISK_FAULTY))
>
> Can a maliciously crafted MD superblock still trigger an out-of-bounds
> memory read through this unvalidated rdev->desc_nr?
>
> [ ... ]
>
>> @@ -1693,6 +1694,13 @@ static void super_90_sync(struct mddev *mddev, struct md_rdev *rdev)
>>   			desc_nr = rdev2->raid_disk;
>>   		else
>>   			desc_nr = next_spare++;
>> +
>> +		if (desc_nr < 0 || desc_nr >= MD_SB_DISKS) {
>> +			pr_warn("md: %s: desc_nr %d out of range for rdev %pg, skipping\n",
>> +				mdname(mddev), desc_nr, rdev2->bdev);
>> +			continue;
>> +		}
>> +
> [Severity: High]
> Does skipping out-of-bounds devices here leave rdev2->desc_nr with a stale
> value?
>
> By using continue to early-exit the loop, we skip the subsequent
> rdev2->desc_nr = desc_nr assignment. Later in this same function, the
> superblock's this_disk field is populated using rdev->desc_nr:
>
>      if (rdev->desc_nr >= 0 && rdev->desc_nr < MD_SB_DISKS)
>          sb->this_disk = sb->disks[rdev->desc_nr];
>
> If the target device was skipped during the rdev_for_each loop, will it
> reuse its stale desc_nr and copy the descriptor of whichever other active
> device took that slot, potentially causing metadata corruption?
This looks like a real issue.
>
>>   		rdev2->desc_nr = desc_nr;
>>   		d = &sb->disks[rdev2->desc_nr];
>>   		nr_disks++;

-- 
Thanks,
Kuai