[PATCH 3/4] md/raid10: fix r10bio devs overflow across reshape

"Chen Cheng" <[email protected]>
Newsgroups gmane.linux.raid
Message-ID <[email protected]>
From: Chen Cheng <[email protected]>

A 4-disk to 5-disk raid10 reshape can complete or free an r10bio that was
allocated before the geometry switch.

The failure was reproduced with a simple write workload while reshaping a
raid10 array from 4 disks to 5 disks, e.g.:

  mdadm -C /dev/md777 -l10 -n4 /dev/sda /dev/sdb /dev/sdc /dev/sdd
  mkfs.ext4 /dev/md777
  mount /dev/md777 /mnt/test
  fsstress -d /mnt/test -n 24000 -p 8 -l 24 &
  mdadm /dev/md777 --add /dev/sde
  mdadm --grow /dev/md777 --raid-devices=5 \
    --backup-file=/tmp/md-reshape-backup

Without this patch, the sequence above can trigger:

  BUG: KASAN: slab-out-of-bounds in free_r10bio+0x1c4/0x260 [raid10]
  Read of size 8 at addr ffff00008c2dfac8 by task ksoftirqd/0/15
  free_r10bio
  raid_end_bio_io
  one_write_done
  raid10_end_write_request

The buggy object was 200 bytes long, which matches an r10bio with space for
only four devs[] entries. However, put_all_bios() and find_bio_disk() walk
r10_bio->devs[] using the current conf->geo.raid_disks value. Once reshape
switches conf->geo.raid_disks from 4 to 5, an old 4-slot r10bio can be
completed or freed as if it had 5 slots, and the walk overruns devs[4].

The same stale-width mismatch can also surface during a 5-disk to 4-disk
reshape.

The same transition also leaves stale-width objects in the active r10bio
pool, so new requests can reuse a 4-slot object after reshape starts unless
the pool is replaced for the new geometry.

Fix this by recording the actual devs[] slot count in each r10bio and using
that count when scanning or freeing the object. Also replace the active
r10bio pool with one sized for the new geometry before reshape switches
layouts. Old-width r10bio objects are freed directly instead of being
returned to a pool that now expects a different width.

A/B validation:

- Without this patch, the 4-disk to 5-disk reshape test triggered the
  KASAN report.
- With this patch, neither the 4-disk to 5-disk nor the 5-disk to 4-disk
  reshape test triggers KASAN.
---
 drivers/md/raid10.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b447903fbdc6..3edde440623a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -234,6 +234,30 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
 	return NULL;
 }
 
+static int reinit_r10bio_pool(struct r10conf *conf, unsigned int nr_devs)
+{
+	struct r10bio_pool *new_pool, *old_pool = conf->r10bio_pool;
+	int ret;
+
+	if (old_pool->nr_devs == nr_devs)
+		return 0;
+
+	new_pool = kzalloc_obj(struct r10bio_pool);
+	if (!new_pool)
+		return -ENOMEM;
+
+	ret = init_r10bio_pool(new_pool, nr_devs);
+	if (ret) {
+		kfree(new_pool);
+		return ret;
+	}
+
+	conf->r10bio_pool = new_pool;
+	mempool_exit(&old_pool->pool);
+	kfree(old_pool);
+	return 0;
+}
+
 static void r10buf_pool_free(void *__r10_bio, void *data)
 {
 	struct r10conf *conf = data;
@@ -268,7 +292,7 @@ static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
 {
 	int i;
 
-	for (i = 0; i < conf->geo.raid_disks; i++) {
+	for (i = 0; i < r10_bio->used_nr_devs; i++) {
 		struct bio **bio = & r10_bio->devs[i].bio;
 		if (!BIO_SPECIAL(*bio))
 			bio_put(*bio);
@@ -285,6 +309,10 @@ static void free_r10bio(struct r10bio *r10_bio)
 	struct r10conf *conf = r10_bio->mddev->private;
 
 	put_all_bios(conf, r10_bio);
+	if (r10_bio->alloc_nr_devs != conf->r10bio_pool->nr_devs) {
+		rbio_pool_free(r10_bio, conf);
+		return;
+	}
 	mempool_free(r10_bio, &conf->r10bio_pool->pool);
 }
 
@@ -365,7 +393,7 @@ static int find_bio_disk(struct r10conf *conf, struct r10bio *r10_bio,
 	int slot;
 	int repl = 0;
 
-	for (slot = 0; slot < conf->geo.raid_disks; slot++) {
+	for (slot = 0; slot < r10_bio->used_nr_devs; slot++) {
 		if (r10_bio->devs[slot].bio == bio)
 			break;
 		if (r10_bio->devs[slot].repl_bio == bio) {
@@ -4416,6 +4444,11 @@ static int raid10_start_reshape(struct mddev *mddev)
 	if (spares < mddev->delta_disks)
 		return -EINVAL;
 
+	raise_barrier(conf, 0);
+	ret = reinit_r10bio_pool(conf, new.raid_disks);
+	if (ret)
+		goto out_lower_barrier;
+
 	conf->offset_diff = min_offset_diff;
 	spin_lock_irq(&conf->device_lock);
 	if (conf->mirrors_new) {
@@ -4433,6 +4466,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 		sector_t size = raid10_size(mddev, 0, 0);
 		if (size < mddev->array_sectors) {
 			spin_unlock_irq(&conf->device_lock);
+			lower_barrier(conf);
 			pr_warn("md/raid10:%s: array size must be reduce before number of disks\n",
 				mdname(mddev));
 			return -EINVAL;
@@ -4443,6 +4477,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 		conf->reshape_progress = 0;
 	conf->reshape_safe = conf->reshape_progress;
 	spin_unlock_irq(&conf->device_lock);
+	lower_barrier(conf);
 
 	if (mddev->delta_disks && mddev->bitmap) {
 		struct mdp_superblock_1 *sb = NULL;
@@ -4527,6 +4562,10 @@ static int raid10_start_reshape(struct mddev *mddev)
 	md_new_event();
 	return 0;
 
+out_lower_barrier:
+	lower_barrier(conf);
+	return ret;
+
 abort:
 	mddev->recovery = 0;
 	spin_lock_irq(&conf->device_lock);
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.