Re: [PATCH] md: add max_disks bound check in older version

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The patch prevents an out-of-bounds write in `super_90_sync()` by adding a bounds check, but leaves an out-of-bounds read unhandled at the end of the function, which can be exploited as a kernel information leak.
- [High] A maliciously crafted version 0.90 superblock on disk can trigger an out-of-bounds read in `super_90_validate()` due to completely missing bounds validation of `this_disk.number` when the superblock is loaded.
--

commit bca793f34834f5048d928e016591b4ea5cf035dc
Author: Aaditya Kansal <aadityakansal390@gmail.com>

md: add max_disks bound check in older version

This commit adds bounds checking for raid_disks in super_90_sync() to prevent
an array-index-out-of-bounds access. It addresses a syzbot report where
a high value written to the raid_disks sysfs attribute triggered the bug.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1693,6 +1693,12 @@ 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 >= MD_SB_DISKS) {
> +			pr_warn("md: raid_disks = %d exceeds max_disks = %d for version 0.90\n",
> +					desc_nr, MD_SB_DISKS);
> +			continue;
> +		}

[Severity: Critical]
This isn't a bug introduced by this patch, but could this continue statement
leave rdev->desc_nr unvalidated, leading to an out-of-bounds read later?

If an attacker provides a malicious block device with a version 0.90
superblock containing a large this_disk.number, super_90_load() assigns
this to rdev->desc_nr. Later, when md_update_sb() calls super_90_sync(),
the newly added bounds check skips updating rdev2->desc_nr because of the
continue statement.

At the end of super_90_sync(), the unvalidated rdev->desc_nr is then
unconditionally used to index into the array:

drivers/md/md.c:super_90_sync() {
    ...
    sb->this_disk = sb->disks[rdev->desc_nr];
    sb->sb_csum = calc_sb_csum(sb);
}

This copies 128 bytes from memory far past the 4KB superblock page into
sb->this_disk. The updated superblock is then written back to disk, which
can persistently leak out-of-bounds kernel memory to the attacker.
Alternatively, if raid_disks is manipulated via sysfs for a new disk,
rdev->desc_nr could remain -1, triggering a UBSAN out-of-bounds read.

[Severity: High]
This is a pre-existing issue, but does super_90_load() completely miss
bounds validation of this_disk.number when the superblock is loaded?

drivers/md/md.c:super_90_load() {
    ...
    rdev->desc_nr = sb->this_disk.number;
    ...
}

When a block device with a crafted version 0.90 superblock is discovered
and assembled, super_90_load() blindly assigns this field without ensuring
it is less than MD_SB_DISKS. Subsequently, super_90_validate() computes
and dereferences an out-of-bounds pointer:

drivers/md/md.c:super_90_validate() {
    ...
    if (sb->disks[rdev->desc_nr].state & (
                (1<<MD_DISK_SYNC) | (1 << MD_DISK_ACTIVE)))
        ...
    desc = sb->disks + rdev->desc_nr;
    if (desc->state & (1<<MD_DISK_FAULTY))
        ...
}

For large values, this dereferences a pointer significantly past the
allocated 4KB superblock page, hitting unmapped memory and causing a kernel
panic. Can bounds validation be added to super_90_load() to prevent this
denial-of-service vector?

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