mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
@ 2026-07-11 10:03 Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 1/3] md: suspend array when sync_action=reshape Chen Cheng
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
  To: linux-raid, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

Hi,

This series fixes slab out-of-bounds accesses in raid10 when reshape changes
the number of raid disks while regular I/O is still reusing r10bio objects
allocated under the previous geometry.

The bug is reproducible with a simple 4-disk to 5-disk reshape under write
load, for example:

  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


KASAN report:

  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


This series addresses the problem in three steps:

  1. ensure the sync_action=reshape caller suspends and locks before start_reshape

  2. resize r10bio_pool when reshape grows raid_disks

  3. reorder the r10bio free flow before bio_endio in the regular and discard
     completion paths

Changes in v7:
   - patch #1, refactor out suspend from retry block
   - patch #3, raid_end_bio_io() refactor to:
        free(r10_bio) -> free(master_bio) -> allow_barrier

Changes in v6:
   - suspend the array in action_store() after flush_work()
   - free r10bio before ending the discard master bio

Changes in v5 (suggested by Yu Kuai):
   - simplify patch 2
   - switch patch 3 from bounding reused r10bio devs[] walks by used_nr_devs
     to reordering the free/endio flow

Changes in v4:
   - make the sync_action=reshape path invoke mddev_suspend_and_lock() before
     calling start_reshape()
   - leave the md-cluster and dm-raid paths unchanged; they still reach
     start_reshape() with the mddev locked but without suspend

Changes in v3:
   - replace freeze_array()/unfreeze_array() in raid10_start_reshape() with
     mddev_suspend_and_lock_nointr()/mddev_unlock_and_resume(); freeze_array()
     can return while retry-list items still hold pool objects, while
     mddev_suspend() provides the correct upper-layer quiesce interface

Changes in v2:
  - add this cover letter
  - convert r10bio_pool to a fixed-size kmalloc mempool
  - rebuild r10bio_pool inside the freeze window before switching live reshape
    geometry
  - switch raid10_quiesce() to freeze_array()/unfreeze_array()


Testing:
  - reproduced the original KASAN slab-out-of-bounds on 4-disk -> 5-disk
    raid10 reshape with fsstress
  - verified that this series fixes that reproducer
  - exercised the 5-disk -> 4-disk reshape direction as well

Thanks,
Chen Cheng



Chen Cheng (3):
  md: suspend array when sync_action=reshape
  md/raid10: resize r10bio_pool for reshape
  md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
    and raid_end_discard_bio()

 drivers/md/md.c     | 17 +++++++++----
 drivers/md/raid10.c | 61 ++++++++++++++++++++++++++++++++-------------
 drivers/md/raid10.h |  2 +-
 3 files changed, 56 insertions(+), 24 deletions(-)

-- 
2.54.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 1/3] md: suspend array when sync_action=reshape
  2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
@ 2026-07-11 10:03 ` Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
  To: linux-raid, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

raid10 needs to resize/swap r10bio_pool when reshape changes
raid_disks, and, don't let new requests keep allocating r10bio
objects from the old pool while that transition is in progress.

suspend and lock array before mddev_start_reshape(), and resume
it on exit.

Other sync_action ops are unchanged.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/md.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..8ee2dcc116eb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5261,25 +5261,32 @@ action_store(struct mddev *mddev, const char *page, size_t len)
 	enum sync_action action;
 
 	if (!mddev->pers || !mddev->pers->sync_request)
 		return -EINVAL;
 
+	action = md_sync_action_by_name(page);
+	if (action == ACTION_RESHAPE) {
+		ret = mddev_suspend(mddev, true);
+		if (ret)
+			return ret;
+	}
 retry:
 	if (work_busy(&mddev->sync_work))
 		flush_work(&mddev->sync_work);
 
 	ret = mddev_lock(mddev);
-	if (ret)
+	if (ret) {
+		if (action == ACTION_RESHAPE)
+			mddev_resume(mddev);
 		return ret;
+	}
 
 	if (work_busy(&mddev->sync_work)) {
 		mddev_unlock(mddev);
 		goto retry;
 	}
 
-	action = md_sync_action_by_name(page);
-
 	/* TODO: mdadm rely on "idle" to start sync_thread. */
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
 		switch (action) {
 		case ACTION_FROZEN:
 			md_frozen_sync_thread(mddev);
@@ -5345,10 +5352,12 @@ action_store(struct mddev *mddev, const char *page, size_t len)
 	sysfs_notify_dirent_safe(mddev->sysfs_action);
 	ret = len;
 
 out:
 	mddev_unlock(mddev);
+	if (action == ACTION_RESHAPE)
+		mddev_resume(mddev);
 	return ret;
 }
 
 static struct md_sysfs_entry md_scan_mode =
 __ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
-- 
2.54.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 2/3] md/raid10: resize r10bio_pool for reshape
  2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 1/3] md: suspend array when sync_action=reshape Chen Cheng
@ 2026-07-11 10:03 ` Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
  To: linux-raid, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

When reshape grows raid_disks, the pool must also switch to new geometry
object size , and allocate a new geometry size pool and replace the old.

But not for shrinking reshape, because regular I/O can still use the
prev geo for sectors that have not crossed reshape_progress yet.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid10.c | 47 ++++++++++++++++++++++++++++++++-------------
 drivers/md/raid10.h |  2 +-
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index cee5a253a281..db68fcc9e9be 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -101,17 +101,27 @@ static void end_reshape(struct r10conf *conf);
 static inline struct r10bio *get_resync_r10bio(struct bio *bio)
 {
 	return get_resync_pages(bio)->raid_bio;
 }
 
-static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
+static inline int calc_r10bio_size(unsigned int raid_disks)
 {
-	struct r10conf *conf = data;
-	int size = offsetof(struct r10bio, devs[conf->geo.raid_disks]);
+	return offsetof(struct r10bio, devs[raid_disks]);
+}
+
+static mempool_t *create_r10bio_pool(unsigned int raid_disks)
+{
+	int size = calc_r10bio_size(raid_disks);
 
-	/* allocate a r10bio with room for raid_disks entries in the
-	 * bios array */
+	return mempool_create_kmalloc_pool(NR_RAID_BIOS, size);
+}
+
+static struct r10bio *alloc_r10bio(unsigned int raid_disks, gfp_t gfp_flags)
+{
+	int size = calc_r10bio_size(raid_disks);
+
+	/* allocate a r10bio sized for current geometry */
 	return kzalloc(size, gfp_flags);
 }
 
 #define RESYNC_SECTORS (RESYNC_BLOCK_SIZE >> 9)
 /* amount of memory to reserve for resync requests */
@@ -135,11 +145,11 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
 	struct bio *bio;
 	int j;
 	int nalloc, nalloc_rp;
 	struct resync_pages *rps;
 
-	r10_bio = r10bio_pool_alloc(gfp_flags, conf);
+	r10_bio = alloc_r10bio(conf->geo.raid_disks, gfp_flags);
 	if (!r10_bio)
 		return NULL;
 
 	if (test_bit(MD_RECOVERY_SYNC, &conf->mddev->recovery) ||
 	    test_bit(MD_RECOVERY_RESHAPE, &conf->mddev->recovery))
@@ -275,11 +285,11 @@ static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
 static void free_r10bio(struct r10bio *r10_bio)
 {
 	struct r10conf *conf = r10_bio->mddev->private;
 
 	put_all_bios(conf, r10_bio);
-	mempool_free(r10_bio, &conf->r10bio_pool);
+	mempool_free(r10_bio, conf->r10bio_pool);
 }
 
 static void put_buf(struct r10bio *r10_bio)
 {
 	struct r10conf *conf = r10_bio->mddev->private;
@@ -1537,11 +1547,11 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
 static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 {
 	struct r10conf *conf = mddev->private;
 	struct r10bio *r10_bio;
 
-	r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
+	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
 
 	r10_bio->master_bio = bio;
 	r10_bio->sectors = sectors;
 
 	r10_bio->mddev = mddev;
@@ -1729,11 +1739,11 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
 		last_stripe_index *= geo->far_copies;
 	end_disk_offset = (bio_end & geo->chunk_mask) +
 				(last_stripe_index << geo->chunk_shift);
 
 retry_discard:
-	r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
+	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
 	r10_bio->mddev = mddev;
 	r10_bio->state = 0;
 	r10_bio->sectors = 0;
 	r10_bio->read_slot = -1;
 	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
@@ -3830,11 +3840,11 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)
 static void raid10_free_conf(struct r10conf *conf)
 {
 	if (!conf)
 		return;
 
-	mempool_exit(&conf->r10bio_pool);
+	mempool_destroy(conf->r10bio_pool);
 	kfree(conf->mirrors);
 	kfree(conf->mirrors_old);
 	kfree(conf->mirrors_new);
 	safe_put_page(conf->tmppage);
 	bioset_exit(&conf->bio_split);
@@ -3877,13 +3887,12 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	if (!conf->tmppage)
 		goto out;
 
 	conf->geo = geo;
 	conf->copies = copies;
-	err = mempool_init(&conf->r10bio_pool, NR_RAID_BIOS, r10bio_pool_alloc,
-			   rbio_pool_free, conf);
-	if (err)
+	conf->r10bio_pool = create_r10bio_pool(conf->geo.raid_disks);
+	if (!conf->r10bio_pool)
 		goto out;
 
 	err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (err)
 		goto out;
@@ -4373,10 +4382,11 @@ static int raid10_start_reshape(struct mddev *mddev)
 	struct geom new;
 	struct r10conf *conf = mddev->private;
 	struct md_rdev *rdev;
 	int spares = 0;
 	int ret;
+	mempool_t *new_pool = NULL;
 
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
 		return -EBUSY;
 
 	if (setup_geo(&new, mddev, geo_start) != conf->copies)
@@ -4409,10 +4419,15 @@ static int raid10_start_reshape(struct mddev *mddev)
 
 	if (spares < mddev->delta_disks)
 		return -EINVAL;
 
 	conf->offset_diff = min_offset_diff;
+	if (mddev->delta_disks > 0) {
+		new_pool = create_r10bio_pool(new.raid_disks);
+		if (!new_pool)
+			return -ENOMEM;
+	}
 	spin_lock_irq(&conf->device_lock);
 	if (conf->mirrors_new) {
 		memcpy(conf->mirrors_new, conf->mirrors,
 		       sizeof(struct raid10_info)*conf->prev.raid_disks);
 		smp_mb();
@@ -4509,10 +4524,14 @@ static int raid10_start_reshape(struct mddev *mddev)
 	mddev->degraded = calc_degraded(conf);
 	spin_unlock_irq(&conf->device_lock);
 	mddev->raid_disks = conf->geo.raid_disks;
 	mddev->reshape_position = conf->reshape_progress;
 	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
+	if (new_pool) {
+		mempool_destroy(conf->r10bio_pool);
+		conf->r10bio_pool = new_pool;
+	}
 
 	clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 	clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
 	clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
 	set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
@@ -4531,10 +4550,12 @@ static int raid10_start_reshape(struct mddev *mddev)
 	smp_wmb();
 	conf->reshape_progress = MaxSector;
 	conf->reshape_safe = MaxSector;
 	mddev->reshape_position = MaxSector;
 	spin_unlock_irq(&conf->device_lock);
+	if (new_pool)
+		mempool_destroy(new_pool);
 	return ret;
 }
 
 /* Calculate the last device-address that could contain
  * any block from the chunk that includes the array-address 's'
diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
index ec79d87fb92f..b711626a5db7 100644
--- a/drivers/md/raid10.h
+++ b/drivers/md/raid10.h
@@ -85,11 +85,11 @@ struct r10conf {
 	int			have_replacement; /* There is at least one
 						   * replacement device.
 						   */
 	wait_queue_head_t	wait_barrier;
 
-	mempool_t		r10bio_pool;
+	mempool_t		*r10bio_pool;
 	mempool_t		r10buf_pool;
 	struct page		*tmppage;
 	struct bio_set		bio_split;
 
 	/* When taking over an array from a different personality, we store
-- 
2.54.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
  2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 1/3] md: suspend array when sync_action=reshape Chen Cheng
  2026-07-11 10:03 ` [PATCH v7 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
@ 2026-07-11 10:03 ` Chen Cheng
  2026-07-11 17:43   ` yu kuai
  2026-07-26 18:56 ` [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape Mykola Marzhan
  2026-07-31  0:40 ` [PATCH v6 " yu kuai
  4 siblings, 1 reply; 12+ messages in thread
From: Chen Cheng @ 2026-07-11 10:03 UTC (permalink / raw)
  To: linux-raid, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

origin flow:

      bio_endio(master_bio);   /* may drop active_io to zero */
      allow_barrier(conf);
      free_r10bio(r10_bio);    /* reads conf->geo, returns to pool */

one scenario is:

  CPU A (softirq, raid_end_bio_io)         CPU B (action_store) --> reshape
  ================================         ===============================
  bio_endio(master_bio)
    md_end_clone_io
      percpu_ref_put -> 0
                                           wait_event wakeup, and,
                                           	mddev_suspend return
                                           raid10_start_reshape:
                                             setup_geo(&conf->geo, new)
                                             ...
                                             mempool_destroy(old_pool)
                                             conf->r10bio_pool = new_pool
  allow_barrier(conf)
  free_r10bio(r10_bio)
    put_all_bios:
      for (i=0; i<conf->geo.raid_disks; i++)
          ==> old obj, new geo, OOB
    mempool_free(r10_bio, conf->r10bio_pool)
          ==> old-geometry obj freed into new pool

so .. fix by reorder the flow:

	free_r10bio(r10_bio)
	allow_barrier(conf)
	bio_endio(master_bio)

raid_end_discard_bio() is exactly the same.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid10.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index db68fcc9e9be..d77f60db7660 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -329,24 +329,27 @@ static void reschedule_retry(struct r10bio *r10_bio)
  */
 static void raid_end_bio_io(struct r10bio *r10_bio)
 {
 	struct bio *bio = r10_bio->master_bio;
 	struct r10conf *conf = r10_bio->mddev->private;
+	unsigned long state = r10_bio->state;
+	bool returned;
 
-	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
-		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
-			bio->bi_status = BLK_STS_IOERR;
+	returned = test_and_set_bit(R10BIO_Returned, &state);
+	if (!returned && !test_bit(R10BIO_Uptodate, &state))
+		bio->bi_status = BLK_STS_IOERR;
+
+	free_r10bio(r10_bio);
+
+	if (!returned)
 		bio_endio(bio);
-	}
 
 	/*
 	 * Wake up any possible resync thread that waits for the device
 	 * to go idle.
 	 */
 	allow_barrier(conf);
-
-	free_r10bio(r10_bio);
 }
 
 /*
  * Update disk head position estimator based on IRQ completion info.
  */
@@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
 		if (!test_bit(R10BIO_Discard, &r10bio->state)) {
 			first_r10bio = (struct r10bio *)r10bio->master_bio;
 			free_r10bio(r10bio);
 			r10bio = first_r10bio;
 		} else {
+			struct bio *master_bio = r10bio->master_bio;
+
 			md_write_end(r10bio->mddev);
-			bio_endio(r10bio->master_bio);
 			free_r10bio(r10bio);
+			bio_endio(master_bio);
 			break;
 		}
 	}
 }
 
-- 
2.54.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
  2026-07-11 10:03 ` [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
@ 2026-07-11 17:43   ` yu kuai
  2026-07-18 12:30     ` Chen Cheng
  0 siblings, 1 reply; 12+ messages in thread
From: yu kuai @ 2026-07-11 17:43 UTC (permalink / raw)
  To: Chen Cheng, linux-raid; +Cc: linux-kernel

Hi,

在 2026/7/11 18:03, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> origin flow:
>
>        bio_endio(master_bio);   /* may drop active_io to zero */
>        allow_barrier(conf);
>        free_r10bio(r10_bio);    /* reads conf->geo, returns to pool */
>
> one scenario is:
>
>    CPU A (softirq, raid_end_bio_io)         CPU B (action_store) --> reshape
>    ================================         ===============================
>    bio_endio(master_bio)
>      md_end_clone_io
>        percpu_ref_put -> 0
>                                             wait_event wakeup, and,
>                                             	mddev_suspend return
>                                             raid10_start_reshape:
>                                               setup_geo(&conf->geo, new)
>                                               ...
>                                               mempool_destroy(old_pool)
>                                               conf->r10bio_pool = new_pool
>    allow_barrier(conf)
>    free_r10bio(r10_bio)
>      put_all_bios:
>        for (i=0; i<conf->geo.raid_disks; i++)
>            ==> old obj, new geo, OOB
>      mempool_free(r10_bio, conf->r10bio_pool)
>            ==> old-geometry obj freed into new pool
>
> so .. fix by reorder the flow:
>
> 	free_r10bio(r10_bio)
> 	allow_barrier(conf)
> 	bio_endio(master_bio)
>
> raid_end_discard_bio() is exactly the same.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
>   drivers/md/raid10.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index db68fcc9e9be..d77f60db7660 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -329,24 +329,27 @@ static void reschedule_retry(struct r10bio *r10_bio)
>    */
>   static void raid_end_bio_io(struct r10bio *r10_bio)
>   {
>   	struct bio *bio = r10_bio->master_bio;
>   	struct r10conf *conf = r10_bio->mddev->private;
> +	unsigned long state = r10_bio->state;
> +	bool returned;
>   
> -	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
> -		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
> -			bio->bi_status = BLK_STS_IOERR;
> +	returned = test_and_set_bit(R10BIO_Returned, &state);
> +	if (!returned && !test_bit(R10BIO_Uptodate, &state))
> +		bio->bi_status = BLK_STS_IOERR;
> +
> +	free_r10bio(r10_bio);
> +
> +	if (!returned)
>   		bio_endio(bio);
> -	}

Apparently this change is not enough, the R10BIO_Returned case still return
the master_bio before freeing r10_bio.

I think you should get an active_io reference while allocating r10_bio to
guarantee that no r10_bio is active while array is suspended.

>   
>   	/*
>   	 * Wake up any possible resync thread that waits for the device
>   	 * to go idle.
>   	 */
>   	allow_barrier(conf);
> -
> -	free_r10bio(r10_bio);
>   }
>   
>   /*
>    * Update disk head position estimator based on IRQ completion info.
>    */
> @@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
>   		if (!test_bit(R10BIO_Discard, &r10bio->state)) {
>   			first_r10bio = (struct r10bio *)r10bio->master_bio;
>   			free_r10bio(r10bio);
>   			r10bio = first_r10bio;
>   		} else {
> +			struct bio *master_bio = r10bio->master_bio;
> +
>   			md_write_end(r10bio->mddev);
> -			bio_endio(r10bio->master_bio);
>   			free_r10bio(r10bio);
> +			bio_endio(master_bio);
>   			break;
>   		}
>   	}
>   }
>   

-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
  2026-07-11 17:43   ` yu kuai
@ 2026-07-18 12:30     ` Chen Cheng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-18 12:30 UTC (permalink / raw)
  To: yukuai, linux-raid; +Cc: linux-kernel

在 2026/7/12 01:43, yu kuai 写道:
>> -	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> -		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> -			bio->bi_status = BLK_STS_IOERR;
>> +	returned = test_and_set_bit(R10BIO_Returned, &state);
>> +	if (!returned && !test_bit(R10BIO_Uptodate, &state))
>> +		bio->bi_status = BLK_STS_IOERR;
>> +
>> +	free_r10bio(r10_bio);
>> +
>> +	if (!returned)
>>    		bio_endio(bio);
>> -	}
> Apparently this change is not enough, the R10BIO_Returned case still return
> the master_bio before freeing r10_bio.
> 



hi Kuai,

thanks for review.

After read the relate part, two scenarios goto raid_end_bio_io:
	a. bio_submit_split_bioset() failed cause early return, and
	b. normal raid-10 completion -> finially call raid_end_bio_io()

for (a), protected by md_handle_request()'s active_io ref life-scope.
	so, even close master io earily , active_io still alive.

for (b), after fix,  free r10bio first. then, close master bio, which
	md_end_clone_io() put active_io refcnt.

both is ok , I think.

thanks.
cheng


> I think you should get an active_io reference while allocating r10_bio to
> guarantee that no r10_bio is active while array is suspended.
> 
>>    
>>    	/*
>>    	 * Wake up any possible resync thread that waits for the device
>>    	 * to go idle.
>>    	 */
>>    	allow_barrier(conf);
>> -
>> -	free_r10bio(r10_bio);
>>    }
>>    
>>    /*
>>     * Update disk head position estimator based on IRQ completion info.
>>     */
>> @@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
>>    		if (!test_bit(R10BIO_Discard, &r10bio->state)) {
>>    			first_r10bio = (struct r10bio *)r10bio->master_bio;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
                   ` (2 preceding siblings ...)
  2026-07-11 10:03 ` [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
@ 2026-07-26 18:56 ` Mykola Marzhan
  2026-07-27  3:07   ` Chen Cheng
  2026-07-27 10:54   ` Mykola Marzhan
  2026-07-31  0:40 ` [PATCH v6 " yu kuai
  4 siblings, 2 replies; 12+ messages in thread
From: Mykola Marzhan @ 2026-07-26 18:56 UTC (permalink / raw)
  To: Chen Cheng; +Cc: linux-raid, linux-kernel, Yu Kuai, Song Liu

Hi Cheng,

Independent confirmation of the bug this series fixes, in case more
evidence helps it land.

While running a KASAN campaign against the llbitmap reshape v2 series
(base 55b77337 — unrelated to your fix) every raid10 grow under IO hit
both faces of the width mismatch:

  BUG: KASAN: slab-out-of-bounds in free_r10bio+0x97/0x110
  Read of size 8 at addr ffff888149044ec8 by task ksoftirqd/2/28
   free_r10bio+0x97/0x110
   raid10_end_write_request+0x260/0x500
   blk_update_request+0x18b/0x6b0
  Allocated by task 232:
   mempool_alloc_noprof+0x108/0x1d0
   raid10_make_request+0x18b/0x1100

  BUG: KASAN: slab-out-of-bounds in raid10_make_request+0x214/0x1100
  Write of size 192 at addr ffff88814da7aa48 by task fio/460
   __asan_memset+0x23/0x50
   raid10_make_request+0x214/0x1100

(192 bytes = 6 x sizeof(struct r10dev) memset into a 4-disk
allocation.)

456 reports across 5 grow cycles (n2, 4->6 disks, four fio randwrite
jobs) on the plain base kernel. The mempool face matches your 2/3
exactly: the 256 preallocated r10bios keep the old width, so any
allocation that dips into the reserve after the grow is a guaranteed
OOB write, not just a read.

The storm is dense enough that raid10 grow under IO is not testable
under KASAN at all right now — which is how we ran into it, and why
we would like to see this land.

We plan to run v7 through the same rig and will follow up with the
result.

Thanks,
Mykola

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-26 18:56 ` [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape Mykola Marzhan
@ 2026-07-27  3:07   ` Chen Cheng
  2026-07-27 10:54   ` Mykola Marzhan
  1 sibling, 0 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-27  3:07 UTC (permalink / raw)
  To: Mykola Marzhan; +Cc: linux-raid, linux-kernel, Yu Kuai, Song Liu

在 2026/7/27 02:56, Mykola Marzhan 写道:
> Hi Cheng,
> 
> Independent confirmation of the bug this series fixes, in case more
> evidence helps it land.
> 
> While running a KASAN campaign against the llbitmap reshape v2 series
> (base 55b77337 — unrelated to your fix) every raid10 grow under IO hit
> both faces of the width mismatch:
> 
>    BUG: KASAN: slab-out-of-bounds in free_r10bio+0x97/0x110
>    Read of size 8 at addr ffff888149044ec8 by task ksoftirqd/2/28
>     free_r10bio+0x97/0x110
>     raid10_end_write_request+0x260/0x500
>     blk_update_request+0x18b/0x6b0
>    Allocated by task 232:
>     mempool_alloc_noprof+0x108/0x1d0
>     raid10_make_request+0x18b/0x1100
> 
>    BUG: KASAN: slab-out-of-bounds in raid10_make_request+0x214/0x1100
>    Write of size 192 at addr ffff88814da7aa48 by task fio/460
>     __asan_memset+0x23/0x50
>     raid10_make_request+0x214/0x1100
> 
> (192 bytes = 6 x sizeof(struct r10dev) memset into a 4-disk
> allocation.)
> 
> 456 reports across 5 grow cycles (n2, 4->6 disks, four fio randwrite
> jobs) on the plain base kernel. The mempool face matches your 2/3
> exactly: the 256 preallocated r10bios keep the old width, so any
> allocation that dips into the reserve after the grow is a guaranteed
> OOB write, not just a read.
> 
> The storm is dense enough that raid10 grow under IO is not testable
> under KASAN at all right now — which is how we ran into it, and why
> we would like to see this land.
> 
> We plan to run v7 through the same rig and will follow up with the
> result.
> 

Hi Mykola:

Thanks to let me know that, and welcome to report the result and if you 
have any idea, welcome to discuss.

thanks,
Cheng.

> Thanks,
> Mykola

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-26 18:56 ` [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape Mykola Marzhan
  2026-07-27  3:07   ` Chen Cheng
@ 2026-07-27 10:54   ` Mykola Marzhan
  2026-07-27 11:17     ` Chen Cheng
  1 sibling, 1 reply; 12+ messages in thread
From: Mykola Marzhan @ 2026-07-27 10:54 UTC (permalink / raw)
  To: Chen Cheng; +Cc: linux-raid, linux-kernel, Yu Kuai, Song Liu

Hi Cheng,

Here are the test results I promised. v7 fixes it.

I used one kernel tree, rebuilt incrementally between the two builds,
so the only difference is your three patches. The array is raid10 n2
with 4 members. The load is four fio jobs: 4k randwrite, O_DIRECT,
iodepth 32. While the load runs, I --add two spares and then
--grow --raid-devices=6. The kernel is KASAN generic with
kasan_multi_shot. Each run is 5 cycles:

  kernel      runs  cycles  grows completed  slab-out-of-bounds
  base           1       5             5/5   128 (in 4 of 5 cycles)
  base + v7      2   5 + 5           10/10     0

All 128 base reports are in free_r10bio(): 64 reads and 64 writes,
each of 8 bytes. There is also a second report site,
raid10_make_request(). A warm-up run on the same base hit it too,
with 2 slab-out-of-bounds there alongside 568 in free_r10bio(). So
both report sites are reachable with this workload.

Every cycle on both kernels logged "md: md0: reshape done." and the
same capacity change, so the grow really did run 10/10 on the patched
kernel, and fio was alive throughout each cycle.

The bug does not fire in every cycle: per-cycle counts on the base
were 0, 40, 8, 56, 24, so the base hit it in 4 of 5 cycles. That rate
is what gives the patched 10/10 zero its meaning.

Tested-by: Mykola Marzhan <mykola@meshstor.io>

One practical note: v7 does not apply to mainline. Patch 2/3 carries
context

  static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)

but e045d6ed33f8 ("md/raid10: fix writes_pending leak on write request
failures") changed __make_request() to return bool. Applying
1/3..3/3 with --fuzz=0:

  8e065a160251 (= e045d6ed33f8^)        applies clean
  e045d6ed33f8                          fails
  55b77337bdd0                          fails
  v7.2-rc4                              fails
  current mainline                      fails

So the numbers above are from 8e065a160251, which is the newest base v7
applies to. A rebase would probably help it get picked up — happy to
re-run the same rig on a v8 if you post one.

Thanks,
Mykola

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-27 10:54   ` Mykola Marzhan
@ 2026-07-27 11:17     ` Chen Cheng
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Cheng @ 2026-07-27 11:17 UTC (permalink / raw)
  To: Mykola Marzhan; +Cc: linux-raid, linux-kernel, Yu Kuai, Song Liu

On 2026/7/27 18:54, Mykola Marzhan wrote:
> Hi Cheng,
> 
> Here are the test results I promised. v7 fixes it.
> 
> I used one kernel tree, rebuilt incrementally between the two builds,
> so the only difference is your three patches. The array is raid10 n2
> with 4 members. The load is four fio jobs: 4k randwrite, O_DIRECT,
> iodepth 32. While the load runs, I --add two spares and then
> --grow --raid-devices=6. The kernel is KASAN generic with
> kasan_multi_shot. Each run is 5 cycles:
> 
>    kernel      runs  cycles  grows completed  slab-out-of-bounds
>    base           1       5             5/5   128 (in 4 of 5 cycles)
>    base + v7      2   5 + 5           10/10     0
> 
> All 128 base reports are in free_r10bio(): 64 reads and 64 writes,
> each of 8 bytes. There is also a second report site,
> raid10_make_request(). A warm-up run on the same base hit it too,
> with 2 slab-out-of-bounds there alongside 568 in free_r10bio(). So
> both report sites are reachable with this workload.
> 
> Every cycle on both kernels logged "md: md0: reshape done." and the
> same capacity change, so the grow really did run 10/10 on the patched
> kernel, and fio was alive throughout each cycle.
> 
> The bug does not fire in every cycle: per-cycle counts on the base
> were 0, 40, 8, 56, 24, so the base hit it in 4 of 5 cycles. That rate
> is what gives the patched 10/10 zero its meaning.
> 
> Tested-by: Mykola Marzhan <mykola@meshstor.io>
> 

Hi Mykola,

I see, really thanks for your test result!

> One practical note: v7 does not apply to mainline. Patch 2/3 carries
> context
> 
>    static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> 
> but e045d6ed33f8 ("md/raid10: fix writes_pending leak on write request
> failures") changed __make_request() to return bool. Applying
> 1/3..3/3 with --fuzz=0:
> 
>    8e065a160251 (= e045d6ed33f8^)        applies clean
>    e045d6ed33f8                          fails
>    55b77337bdd0                          fails
>    v7.2-rc4                              fails
>    current mainline                      fails
> 

Got it, I must misuse a unclean branch.

> So the numbers above are from 8e065a160251, which is the newest base v7
> applies to. A rebase would probably help it get picked up — happy to
> re-run the same rig on a v8 if you post one.
> 

I don't want to waste your time, I would re-run reproducer before I send.

Reproducer Link: 
https://github.com/chencheng-fnnas/reproducer/blob/main/repro-raid10-4to5-reshape-r10bio-oob.sh

Thanks again,
Cheng.

> Thanks,
> Mykola

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
                   ` (3 preceding siblings ...)
  2026-07-26 18:56 ` [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape Mykola Marzhan
@ 2026-07-31  0:40 ` yu kuai
  2026-07-31 10:13   ` Mykola Marzhan
  4 siblings, 1 reply; 12+ messages in thread
From: yu kuai @ 2026-07-31  0:40 UTC (permalink / raw)
  To: Chen Cheng, linux-raid, yu kuai; +Cc: linux-kernel, Mykola Marzhan

Hi,

在 2026/7/11 18:03, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> Hi,
>
> This series fixes slab out-of-bounds accesses in raid10 when reshape changes
> the number of raid disks while regular I/O is still reusing r10bio objects
> allocated under the previous geometry.
>
> The bug is reproducible with a simple 4-disk to 5-disk reshape under write
> load, for example:
>
>    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
>
>
> KASAN report:
>
>    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
>
>
> This series addresses the problem in three steps:
>
>    1. ensure the sync_action=reshape caller suspends and locks before start_reshape
>
>    2. resize r10bio_pool when reshape grows raid_disks
>
>    3. reorder the r10bio free flow before bio_endio in the regular and discard
>       completion paths
>
> Changes in v7:
>     - patch #1, refactor out suspend from retry block
>     - patch #3, raid_end_bio_io() refactor to:
>          free(r10_bio) -> free(master_bio) -> allow_barrier
>
> Changes in v6:
>     - suspend the array in action_store() after flush_work()
>     - free r10bio before ending the discard master bio
>
> Changes in v5 (suggested by Yu Kuai):
>     - simplify patch 2
>     - switch patch 3 from bounding reused r10bio devs[] walks by used_nr_devs
>       to reordering the free/endio flow
>
> Changes in v4:
>     - make the sync_action=reshape path invoke mddev_suspend_and_lock() before
>       calling start_reshape()
>     - leave the md-cluster and dm-raid paths unchanged; they still reach
>       start_reshape() with the mddev locked but without suspend
>
> Changes in v3:
>     - replace freeze_array()/unfreeze_array() in raid10_start_reshape() with
>       mddev_suspend_and_lock_nointr()/mddev_unlock_and_resume(); freeze_array()
>       can return while retry-list items still hold pool objects, while
>       mddev_suspend() provides the correct upper-layer quiesce interface
>
> Changes in v2:
>    - add this cover letter
>    - convert r10bio_pool to a fixed-size kmalloc mempool
>    - rebuild r10bio_pool inside the freeze window before switching live reshape
>      geometry
>    - switch raid10_quiesce() to freeze_array()/unfreeze_array()
>
>
> Testing:
>    - reproduced the original KASAN slab-out-of-bounds on 4-disk -> 5-disk
>      raid10 reshape with fsstress
>    - verified that this series fixes that reproducer
>    - exercised the 5-disk -> 4-disk reshape direction as well
>
> Thanks,
> Chen Cheng
>
>
>
> Chen Cheng (3):
>    md: suspend array when sync_action=reshape
>    md/raid10: resize r10bio_pool for reshape
>    md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
>      and raid_end_discard_bio()
>
>   drivers/md/md.c     | 17 +++++++++----
>   drivers/md/raid10.c | 61 ++++++++++++++++++++++++++++++++-------------
>   drivers/md/raid10.h |  2 +-
>   3 files changed, 56 insertions(+), 24 deletions(-)

Applied to md-7.3 with a few changes, please check the latest md-7.3 branch:

   - Patch 2: removed the redundant if (new_pool) before mempool_destroy(new_pool) to fix the
     strict checkpatch warning.

   - Patch 3: rewrote raid_end_bio_io() minimally so the existing R10BIO_* bit-test lines stay in
     place and checkpatch no longer flags CamelCase additions. This also preserves setting
     R10BIO_Returned in r10_bio->state before freeing.

   - Patch 3 commit message: corrected the listed final order to free_r10bio() -> bio_endio() ->
     allow_barrier().

>
-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
  2026-07-31  0:40 ` [PATCH v6 " yu kuai
@ 2026-07-31 10:13   ` Mykola Marzhan
  0 siblings, 0 replies; 12+ messages in thread
From: Mykola Marzhan @ 2026-07-31 10:13 UTC (permalink / raw)
  To: Yu Kuai; +Cc: Chen Cheng, linux-raid, linux-kernel, Song Liu

Hi Kuai,

Thank you for applying the series. I tested latest md-7.3 branch.

Good news: md-7.3 is completely clean on 4 -> 6 and 4 -> 5 grows.

Tested-by: Mykola Marzhan <mykola@meshstor.io>

Thanks,
Mykola

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-07-31 10:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-11 10:03 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
2026-07-11 10:03 ` [PATCH v7 1/3] md: suspend array when sync_action=reshape Chen Cheng
2026-07-11 10:03 ` [PATCH v7 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
2026-07-11 10:03 ` [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
2026-07-11 17:43   ` yu kuai
2026-07-18 12:30     ` Chen Cheng
2026-07-26 18:56 ` [PATCH v7 0/3] md/raid10: fix r10bio width mismatches across reshape Mykola Marzhan
2026-07-27  3:07   ` Chen Cheng
2026-07-27 10:54   ` Mykola Marzhan
2026-07-27 11:17     ` Chen Cheng
2026-07-31  0:40 ` [PATCH v6 " yu kuai
2026-07-31 10:13   ` Mykola Marzhan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®