[PATCH] dm-raid: bound array_position before indexing failed_devices

[PATCH] dm-raid: bound array_position before indexing failed_devices

[Jay Vadayath]

super_init_validation() reads role = le32_to_cpu(sb2->array_position)
out of an on-disk dm_raid_superblock and then uses it as a bit index into
the failed_devices bitmap via test_bit(role, (void *)failed_devices).
failed_devices is a uint64_t[DISKS_ARRAY_ELEMS] (DISKS_ARRAY_ELEMS == 4,
i.e. 256 bits) living on the stack. The only guard is role < 0, so any
on-disk array_position >= 256 makes test_bit() read past the end of the
stack array (array_position == 256 reads failed_devices[4], 8 bytes past
the object). For a raid1 set the role != raid_disk reorder branch falls
through to the same test_bit().

A crafted dm-raid superblock therefore causes a stack out-of-bounds read
during table load. Reachable by anyone able to issue a dmsetup create
with attacker-controlled metadata (CAP_SYS_ADMIN in the target
namespace); the crash fires in the constructor / analyse_superblocks
path, before any md_run or array sizing.

  BUG: KASAN: stack-out-of-bounds in super_validate+0x1cd0/0x3390
  Read of size 8 at addr ffff888007b17828 by task poc/152
  Call Trace:
   dump_stack_lvl+0x64/0x80
   print_report+0xd0/0x630
   kasan_report+0xe5/0x120
   kasan_check_range+0x105/0x1b0
   super_validate+0x1cd0/0x3390        (super_init_validation, inlined)
   raid_ctr+0x2c4b/0xa0f0
   dm_table_add_target+0x66f/0xf80
   table_load+0x2ee/0x780
   ctl_ioctl+0x362/0x9f0
   dm_ctl_ioctl+0x9/0x10
   __x64_sys_ioctl+0x134/0x1c0
   do_syscall_64+0xe2/0x4b0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

role is a device position that must refer to an actual member of the
array, so reject anything outside [0, rs->raid_disks) instead of only the
negative case. rs->raid_disks is always <= the 256-bit capacity of
failed_devices, so this both closes the out-of-bounds access and matches
the intended semantics.

This bug was discovered by Artiphishell's vTriage pipeline, which
generated a crafted raid1 superblock (magic "DmRd", compat_features=1,
num_devices=2, array_position=256, level=1) and a dmsetup-create
reproducer that reliably triggers the KASAN report on an unpatched kernel
(the precursor DMINFO "raid device #256 now at position #0" is visible in
the serial log just before the splat). The fix below was drafted with the
Claude coding assistant; a userspace reproducer is available on request.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jay Vadayath <jay@artiphishell.com>
---
 drivers/md/dm-raid.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 8f5a5e1342a9..d497d7c68596 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -2420,7 +2420,8 @@ static int super_init_validation(struct raid_set *rs, struct md_rdev *rdev)
 		 */
 		if (!test_bit(FirstUse, &r->flags) && (r->raid_disk >= 0)) {
 			role = le32_to_cpu(sb2->array_position);
-			if (role < 0)
+			/* Bound @role before it indexes the failed_devices stack bitmap. */
+			if (role < 0 || role >= rs->raid_disks)
 				continue;
 
 			if (role != r->raid_disk) {
-- 
2.51.2

Re: [PATCH] dm-raid: bound array_position before indexing failed_devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dm-raid: wrong role bound and continue bypass in super_init_validation()
--

commit 46b04454594856163e75cd4dccd01fd7ff3efea6
Author: Jay Vadayath <jay@artiphishell.com>

dm-raid: bound array_position before indexing failed_devices

This commit bounds the on-disk array_position (role) by rs->raid_disks in
super_init_validation() before using it as a bit index into the on-stack
failed_devices bitmap.

> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index ac019a0f5a929..b973181bdce24 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -2420,7 +2420,8 @@ static int super_init_validation(struct raid_set *rs, struct md_rdev *rdev)
>  		 */
>  		if (!test_bit(FirstUse, &r->flags) && (r->raid_disk >= 0)) {
>  			role = le32_to_cpu(sb2->array_position);
> -			if (role < 0)
> +			/* Bound @role before it indexes the failed_devices stack bitmap. */
> +			if (role < 0 || role >= rs->raid_disks)
>  				continue;

[Severity: High]
Does using continue when role >= rs->raid_disks in
drivers/md/dm-raid.c:super_init_validation() introduce two regressions during
table load (dm_ctl_ioctl() -> table_load() -> dm_table_add_target() ->
raid_ctr() -> analyse_superblocks() -> super_validate() ->
super_init_validation())?

First, the commit message states that this change rejects anything outside
[0, rs->raid_disks), but executing continue only skips the remainder of the
per-rdev loop body and allows super_init_validation() to return 0:

drivers/md/dm-raid.c:super_init_validation() {
    ...
		if (!test_bit(FirstUse, &r->flags) && (r->raid_disk >= 0)) {
			role = le32_to_cpu(sb2->array_position);
			/* Bound @role before it indexes the failed_devices stack bitmap. */
			if (role < 0 || role >= rs->raid_disks)
				continue;

			if (role != r->raid_disk) {
				...
				} else if (!(rs_is_raid10(rs) && rt_is_raid0(rs->raid_type)) &&
					   !(rs_is_raid0(rs) && rt_is_raid10(rs->raid_type)) &&
					   !rt_is_raid1(rs->raid_type)) {
					rs->ti->error = "Cannot change device positions in raid set";
					return -EINVAL;
				}
    ...
}

Because r->raid_disk is in [0, rs->raid_disks), any device with
role >= rs->raid_disks has role != r->raid_disk. Previously, on raid levels
that forbid device reordering (such as raid4/5/6 or raid10 far/offset), a
device with role in [rs->raid_disks, 255] entered the role != r->raid_disk
block and failed validation with return -EINVAL. With this continue, shouldn't
an out-of-bounds role return -EINVAL instead of silently bypassing both the
device reordering check and the failed_devices check?

Second, is rs->raid_disks the right upper bound for role here? In
super_init_validation(), role is the device's previous position in the
on-disk array of size mddev->raid_disks (le32_to_cpu(sb->num_devices)), and
failed_devices is indexed by that previous role, whereas rs->raid_disks is
the device count of the newly loaded table.

dm-raid allows loading tables with rs->raid_disks < mddev->raid_disks when
removing legs from a raid1 array without delta_disks or when removing mirror
copies from a raid10_near array:

drivers/md/dm-raid.c:rs_prepare_reshape() {
	if (rs_is_raid10(rs)) {
		if (rs->raid_disks != mddev->raid_disks &&
		    __is_raid10_near(mddev->layout) &&
		    rs->raid10_copies &&
		    rs->raid10_copies != __raid10_near_copies(mddev->layout)) {
			...
			__reorder_raid_disk_indexes(rs);
			...
		}
	...
	} else if (rs_is_raid1(rs)) {
		if (rs->delta_disks) {
			...
		} else {
			/* Process raid1 without delta_disks */
			mddev->raid_disks = rs->raid_disks;
			reshape = false;
		}
	}
}

When a non-trailing leg or mirror copy is removed so that rs->raid_disks <
mddev->raid_disks (for example, removing leg 0 from a 3-disk raid1 array so
old device 2 moves to r->raid_disk = 1 with rs->raid_disks = 2, or reducing a
6-disk 3-copy raid10_near set to a 4-disk 2-copy set so old device 4 moves to
r->raid_disk = 3 with rs->raid_disks = 4), surviving devices legitimately
have rs->raid_disks <= role < mddev->raid_disks.

If role >= rs->raid_disks takes the continue branch, won't
super_init_validation() skip updating sb2->array_position for raid10_near and
skip testing failed_devices to set Faulty on a returning failed device,
preventing super_validate() from clearing In_sync and resetting
recovery_offset to 0?

>  
>  			if (role != r->raid_disk) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022181934.35685-1-jay@artiphishell.com?part=1