Re: [PATCH v2] md: reject raid_disks >= MD_SB_DISKS in md_set_array_info()

[yu kuai]

Hi，

在 2026/9/19 19:25, Hui Peng 写道:
> md_set_array_info() copies info->raid_disks straight into
> mddev->raid_disks without any upper bound, while also setting
> mddev->max_disks to MD_SB_DISKS for persistent arrays. The resulting
> mddev is internally inconsistent: raid_disks can be far larger than the
> number of devices a v0.90 superblock is able to describe.
>
> super_90_sync() lays the device table out inside the single page that
> holds the v0.90 superblock and picks slot numbers starting at
> mddev->raid_disks:
>
> 	next_spare = mddev->raid_disks;
> 	...
> 	desc_nr = next_spare++;
> 	rdev2->desc_nr = desc_nr;
> 	d = &sb->disks[rdev2->desc_nr];
>
> sb->disks[] only has MD_SB_DISKS (27) entries, so an
> ioctl(SET_ARRAY_INFO) with raid_disks = 1000 followed by any operation
> that writes the superblock walks tens of kilobytes past the end of the
> superblock page:
>
>   ==================================================================
>   BUG: KASAN: use-after-free in super_90_sync+0x1c5e/0x20c0
>   Write of size 4 at addr 0xffff888009623c00 by task init/1
>
>   CPU: 1 UID: 0 PID: 1 Comm: init Tainted: G      D W    7.3.0-rc3-g5dd1818b15d9 #3 PREEMPT(lazy)
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x4d/0x70
>    print_report+0x153/0x4c6
>    kasan_report+0xda/0x110
>    super_90_sync+0x1c5e/0x20c0
>    md_update_sb+0x850/0x1e90
>    new_level_store+0x11a/0x190
>    md_attr_store+0x12f/0x270
>    kernfs_fop_write_iter+0x323/0x4e0
>    vfs_write+0x54f/0xde0
>    ksys_write+0xfd/0x200
>    do_syscall_64+0xda/0x4b0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>
>   ==================================================================
>
> Reject the request before mddev is modified. Non-persistent arrays do
> not write a v0.90 superblock, so the MD_SB_DISKS limit is only applied
> when info->not_persistent is clear.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>
> ---
> v2: Add a Fixes: tag. v1 said the check predated the history I had; I
> now have the full tree, and it really does go back to the initial git
> import - set_array_info() in v2.6.12-rc2 already did
>
> 	mddev->raid_disks    = info->raid_disks;
> 	mddev->max_disks     = MD_SB_DISKS;
>
> with no bound, and super_90_sync() already seeded next_spare from
> mddev->raid_disks and stored into sb->disks[rdev2->desc_nr]. So
> 1da177e4c3f4 ("Linux-2.6.12-rc2") it is. The later commits that blame
> points at are not introducers: 7e0adbfc20c5 ("md: factor out
> md_set_array_info") only renames set_array_info(), and 86e6ffdd243a
> ("md: extend md sysfs support to component devices") only restructured
> how desc_nr is chosen.
>
> The reason the MD_SB_DISKS limit is gated on !info->not_persistent is
> 1b3bae49fba5 ("md: don't impose the MD_SB_DISKS limit on arrays without
> metadata"), which moved max_disks = MD_SB_DISKS inside
> if (mddev->persistent). Arrays without metadata never write a v0.90
> superblock, so the limit must not apply to them.
>
> Reproduced on Linux 7.3.0-rc3 (5dd1818b15d9) with KASAN under QEMU:
> create an md array over a loop device, issue
> ioctl(fd, SET_ARRAY_INFO) with raid_disks = 1000, then write to
> /sys/block/md0/md/new_level to force md_update_sb(). With this patch the
> ioctl fails with -EINVAL and no splat is produced.
>
> Note that raid_disks_store() has a related but weaker check: it bounds n
> against mddev->max_disks, which is still 0 before any superblock has
> been loaded. I have deliberately not touched it here, since an
> unconditional MD_SB_DISKS limit there would break v1.x arrays with more
> than MD_SB_DISKS members. It may deserve a separate fix - input from
> the maintainers would be welcome.

v1.x array do not have this problem, and it's not acceptable to limit v1.x
array with MD_SB_DISKS. How about following solution:

If metadata is set as 0.9 before raid_disk_store, you can check max_disks in
raid_disk_store; If metadata is not set yet, check later when metadata is selected.

>
>   drivers/md/md.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7926,6 +7926,10 @@ int md_set_array_info(struct mddev *mdde
>   		mddev->ctime         = ktime_get_real_seconds();
>   		return 0;
>   	}
> +	if (info->raid_disks <= 0 ||
> +	    (!info->not_persistent && info->raid_disks >= MD_SB_DISKS))
> +		return -EINVAL;
> +
>   	mddev->major_version = MD_MAJOR_VERSION;
>   	mddev->minor_version = MD_MINOR_VERSION;
>   	mddev->patch_version = MD_PATCHLEVEL_VERSION;

-- 
Thanks,
Kuai