All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] md: reject v0.90 arrays with too many devices
@ 2026-08-20 21:07 Jérémy Jean
  2026-08-20 21:24 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Jérémy Jean @ 2026-08-20 21:07 UTC (permalink / raw)
  To: song, yukuai
  Cc: magiclinan, xiao, linux-raid, linux-kernel, Jérémy Jean

The v0.90 superblock has room for MD_SB_DISKS device descriptors, but
super_90_load() only rejects nonpositive raid_disks values. A crafted
superblock can therefore copy a larger value into mddev->raid_disks. When
metadata is next updated, super_90_sync() walks sb->disks[] up to that
count and reads and writes beyond the 4 KiB superblock buffer.

A checksum-valid RAID1 member advertising 64 devices can be assembled by
mdadm --incremental --run. The first write to the array then produces
out-of-bounds writes from super_90_sync() under KASAN.

Reject device counts that cannot be represented by the v0.90 format while
loading the superblock.

Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 drivers/md/md.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..873a0e5fce7d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1398,7 +1398,7 @@ 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)) {
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] md: reject v0.90 arrays with too many devices
  2026-08-20 21:07 [PATCH] md: reject v0.90 arrays with too many devices Jérémy Jean
@ 2026-08-20 21:24 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-20 21:24 UTC (permalink / raw)
  To: Jérémy Jean; +Cc: linux-raid, yukuai

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

Pre-existing issues:
- [High] The bounds check on `raid_disks` is incomplete and allows out-of-bounds writes in `super_90_sync()`.
- [High] Missing bounds check on `sb->this_disk.number` allows arbitrary out-of-bounds memory reads leading to kernel panic.
--

--- Patch [1]: [PATCH] md: reject v0.90 arrays with too many devices ---
commit 29dd6c25e23c9e9c7bcdc332429c7ae58ceccde2
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Subject: md: reject v0.90 arrays with too many devices

The commit updates super_90_load to reject arrays where the raid_disks
value exceeds MD_SB_DISKS. This prevents out-of-bounds writes during
superblock metadata updates when processing a crafted v0.90 format
superblock.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..154ee5a65cb79 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1393,7 +1393,7 @@ 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)

[Severity: High]
This isn't a bug introduced by this patch, but does allowing sb->raid_disks
to equal MD_SB_DISKS (which is 27) leave the array vulnerable to an
out-of-bounds write?

If sb->raid_disks is exactly MD_SB_DISKS, and the array has missing active
disks but contains spare or faulty devices, super_90_sync() will initialize
next_spare to mddev->raid_disks (27).

For each spare or faulty device, it unconditionally assigns
desc_nr = next_spare++ and writes to sb->disks[desc_nr]:

drivers/md/md.c:super_90_sync() {
    ...
    desc_nr = next_spare++;
    ...
    d = &sb->disks[rdev2->desc_nr];
    ...
}

Since sb->disks has exactly 27 elements, this means next_spare starts out
of bounds. Successive spare devices could cause writes to sb->disks[28],
sb->disks[29], exceeding the 4KB superblock page. Should this validation
ensure raid_disks is strictly less than MD_SB_DISKS?

[Severity: High]
This is also a pre-existing issue, but further down in super_90_load(),
sb->this_disk.number is extracted into rdev->desc_nr:

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

Is it possible for an untrusted value to be extracted here without verifying
it is strictly less than MD_SB_DISKS?

Later, when array assembly calls md_run(), it executes super_90_validate()
which uses this unvalidated value as an array index:

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

Since rdev->desc_nr can be arbitrarily large, could this result in reading
memory far beyond the sb_page and trigger a kernel panic if it lands on an
unmapped page?

>  		goto abort;
>  
>  	if (md_csum_fold(calc_sb_csum(sb)) != md_csum_fold(sb->sb_csum)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820210703.3035608-2-Jeremy.Jean@oss.cyber.gouv.fr?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-20 21:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 21:07 [PATCH] md: reject v0.90 arrays with too many devices Jérémy Jean
2026-08-20 21:24 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.