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

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

[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.

Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
No Fixes: tag: the missing check appears to predate the git history I
have available, so I would rather not guess at a commit.

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.

 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;
-- 
2.43.0

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

[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@gmail.com>
---
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.

 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;
-- 
2.43.0