[PATCH] md/raid0: don't BUG() on a bio that lands off the end of the array

Palla Raghunath posted 1 patch 20 hours ago
drivers/md/raid0.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
[PATCH] md/raid0: don't BUG() on a bio that lands off the end of the array
Posted by Palla Raghunath 20 hours ago
find_zone() looks for the strip zone holding a given sector and BUG()s if
it can't find one.  That is only a safe assumption while the capacity the
array advertises matches what the zones actually cover, and a personality
takeover can break exactly that.

level_store() lets you switch a live array to a different personality.
The new ->run() recalculates mddev->array_sectors, but level_store() never
tells the gendisk about it - it is the one place in md that changes the
array size without a set_capacity_and_notify(), which do_md_run(),
array_size_store(), update_size(), md_reap_sync_thread() and linear_add()
all do.  So when a takeover makes the array smaller, the block device goes
on advertising the old size and I/O sails off the end of the new mapping.

syzbot got there with a raid1 that had no metadata.  Such a member comes in
through md_import_device(dev, -1, -1), which never sets rdev->sectors, and
the check in bind_rdev_to_array() that keeps mddev->dev_sectors inside the
member is skipped precisely when rdev->sectors is 0:

	if (!test_bit(Journal, &rdev->flags) &&
	    rdev->sectors &&
	    (mddev->dev_sectors == 0 || rdev->sectors < mddev->dev_sectors)) {

md/component_size had set dev_sectors to 2048, so raid1 came up with
array_sectors = 2048 and the disk got a 2048 sector capacity.  Writing
"raid0" to md/level then took the array over: create_strip_zones() rounded
rdev->sectors (0) down to a chunk multiple, so the single strip zone ended
at 0 and raid0_size() returned 0.  The capacity stayed at 2048, so the
partition scan on the next open of /dev/md0 read sector 0 and fell over:

  kernel BUG at drivers/md/raid0.c:318!
  RIP: 0010:find_zone drivers/md/raid0.c:318 [inline]
  RIP: 0010:raid0_map_submit_bio drivers/md/raid0.c:568 [inline]
  RIP: 0010:raid0_make_request+0xf17/0x10b0 drivers/md/raid0.c:626
  Call Trace:
   <TASK>
   md_handle_request+0xb0a/0xe60 drivers/md/md.c:417
   __submit_bio+0x27f/0x340 block/blk-core.c:681
   submit_bio_noacct_nocheck+0x4e8/0xa40 block/blk-core.c:792
   block_read_full_folio+0x7a6/0x810 fs/buffer.c:2373
   read_part_sector+0xb6/0x2b0 block/partitions/core.c:724
   adfspart_check_ICS+0xb1/0x960 block/partitions/acorn.c:357
   bdev_disk_changed+0x851/0x17a0 block/partitions/core.c:695
   blkdev_get_whole+0x372/0x510 block/bdev.c:793
   bdev_open+0x324/0xd70 block/bdev.c:1002
   blkdev_open+0x461/0x600 block/fops.c:674
   __x64_sys_openat+0x138/0x170 fs/open.c:1434

Just refreshing the capacity in level_store() is not enough on its own.
If the size is managed externally - md/array_size written while the array
was stopped, which slips past array_size_store()'s -E2BIG check -
md_set_array_sectors() does nothing, array_sectors keeps the larger value
and we BUG() all the same.  level_store() cannot turn the takeover away
the way do_md_run() does either, because ->takeover() has already run and
the old personality has already been freed.

md-linear has always just failed a sector outside its mapping instead of
asserting, so do the same here: let find_zone() return NULL and have its
two callers report the bio and end it with an I/O error.

Tested both ways into this state - a takeover that leaves the array
zero-length, and one where md/array_size pins array_sectors at 2048.
Before, each panics in find_zone(); after, the read just fails:

  md/raid0:md0: sector 0 out of bounds, array size 0
  md/raid0:md0: sector 0 out of bounds, array size 2048

Reported-by: syzbot+422b372cabfbaa528725@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=422b372cabfbaa528725
Fixes: 9af204cf720c ("md: Add support for Raid5->Raid0 and Raid10->Raid0 takeover")
Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
---
 drivers/md/raid0.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 35e103f0c2c3..04c022d74fae 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -301,6 +301,7 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
 
 /* Find the zone which holds a particular offset
  * Update *sectorp to be an offset in that zone
+ * Returns NULL if the sector is off the end of the array.
  */
 static struct strip_zone *find_zone(struct r0conf *conf,
 				    sector_t *sectorp)
@@ -315,7 +316,16 @@ static struct strip_zone *find_zone(struct r0conf *conf,
 				*sectorp = sector - z[i-1].zone_end;
 			return z + i;
 		}
-	BUG();
+	return NULL;
+}
+
+static void raid0_out_of_bounds(struct mddev *mddev, struct bio *bio)
+{
+	pr_err_ratelimited("md/raid0:%s: sector %llu out of bounds, array size %llu\n",
+			   mdname(mddev),
+			   (unsigned long long)bio->bi_iter.bi_sector,
+			   (unsigned long long)mddev->array_sectors);
+	bio_io_error(bio);
 }
 
 /*
@@ -471,6 +481,10 @@ static void raid0_handle_discard(struct mddev *mddev, struct bio *bio)
 
 	orig_start = start;
 	zone = find_zone(conf, &start);
+	if (!zone) {
+		raid0_out_of_bounds(mddev, bio);
+		return;
+	}
 
 	if (bio_end_sector(bio) > zone->zone_end) {
 		bio = bio_submit_split_bioset(bio,
@@ -566,6 +580,11 @@ static void raid0_map_submit_bio(struct mddev *mddev, struct bio *bio)
 	md_account_bio(mddev, &bio);
 
 	zone = find_zone(mddev->private, &sector);
+	if (!zone) {
+		raid0_out_of_bounds(mddev, bio);
+		return;
+	}
+
 	switch (conf->layout) {
 	case RAID0_ORIG_LAYOUT:
 		tmp_dev = map_sector(mddev, zone, bio_sector, &sector);
-- 
2.34.1