Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
@ 2026-06-23 12:38 Chen Cheng
  2026-06-23 12:38 ` [PATCH v6 1/3] md: suspend array when sync_action=reshape Chen Cheng
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, 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 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] 6+ messages in thread

* [PATCH v6 1/3] md: suspend array when sync_action=reshape
  2026-06-23 12:38 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
@ 2026-06-23 12:38 ` Chen Cheng
  2026-06-23 12:55   ` sashiko-bot
  2026-06-23 12:38 ` [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
  2026-06-23 12:38 ` [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
  2 siblings, 1 reply; 6+ messages in thread
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, 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 | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..1377c407614c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5261,25 +5261,29 @@ 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);
 retry:
 	if (work_busy(&mddev->sync_work))
 		flush_work(&mddev->sync_work);
 
-	ret = mddev_lock(mddev);
+	ret = (action == ACTION_RESHAPE) ?
+		mddev_suspend_and_lock(mddev) :
+		mddev_lock(mddev);
 	if (ret)
 		return ret;
 
 	if (work_busy(&mddev->sync_work)) {
-		mddev_unlock(mddev);
+		if (action == ACTION_RESHAPE)
+			mddev_unlock_and_resume(mddev);
+		else
+			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);
@@ -5344,11 +5348,14 @@ action_store(struct mddev *mddev, const char *page, size_t len)
 	md_wakeup_thread(mddev->thread);
 	sysfs_notify_dirent_safe(mddev->sysfs_action);
 	ret = len;
 
 out:
-	mddev_unlock(mddev);
+	if (action == ACTION_RESHAPE)
+		mddev_unlock_and_resume(mddev);
+	else
+		mddev_unlock(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 related	[flat|nested] 6+ messages in thread

* [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape
  2026-06-23 12:38 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
  2026-06-23 12:38 ` [PATCH v6 1/3] md: suspend array when sync_action=reshape Chen Cheng
@ 2026-06-23 12:38 ` Chen Cheng
  2026-06-23 13:00   ` sashiko-bot
  2026-06-23 12:38 ` [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng
  2 siblings, 1 reply; 6+ messages in thread
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, 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 | 44 +++++++++++++++++++++++++++++++++-----------
 drivers/md/raid10.h |  2 +-
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index cee5a253a281..d740744a9746 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -101,14 +101,25 @@ 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);
+
+	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 with room for raid_disks entries in the
 	 * bios array */
 	return kzalloc(size, gfp_flags);
 }
@@ -135,11 +146,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 +286,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 +1548,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 +1740,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 +3841,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 +3888,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 +4383,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 +4420,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 +4525,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 +4551,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 related	[flat|nested] 6+ messages in thread

* [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
  2026-06-23 12:38 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
  2026-06-23 12:38 ` [PATCH v6 1/3] md: suspend array when sync_action=reshape Chen Cheng
  2026-06-23 12:38 ` [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
@ 2026-06-23 12:38 ` Chen Cheng
  2 siblings, 0 replies; 6+ messages in thread
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, 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_io)

raid_end_discard_bio() is exactly the same.

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

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d740744a9746..e44a9b6e95c7 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -330,24 +330,25 @@ 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;
 
-	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
-		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
-			bio->bi_status = BLK_STS_IOERR;
-		bio_endio(bio);
-	}
+	free_r10bio(r10_bio);
 
 	/*
 	 * Wake up any possible resync thread that waits for the device
 	 * to go idle.
 	 */
 	allow_barrier(conf);
 
-	free_r10bio(r10_bio);
+	if (!test_and_set_bit(R10BIO_Returned, &state)) {
+		if (!test_bit(R10BIO_Uptodate, &state))
+			bio->bi_status = BLK_STS_IOERR;
+		bio_endio(bio);
+	}
 }
 
 /*
  * Update disk head position estimator based on IRQ completion info.
  */
@@ -1580,13 +1581,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 related	[flat|nested] 6+ messages in thread

* Re: [PATCH v6 1/3] md: suspend array when sync_action=reshape
  2026-06-23 12:38 ` [PATCH v6 1/3] md: suspend array when sync_action=reshape Chen Cheng
@ 2026-06-23 12:55   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-23 12:55 UTC (permalink / raw)
  To: Chen Cheng; +Cc: linux-raid, yukuai

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent calls to mddev_suspend() corrupt current->flags due to a shared mddev->noio_flag.
- [High] TOCTOU race on mddev->pers in action_store leading to NULL pointer dereference.
- [High] Unprotected access to mddev->pers inside mddev_suspend leading to TOCTOU and potential Use-After-Free.
--

commit 480ff61edd64974d75f3192274f5151dada9f98c
Author: Chen Cheng <chencheng@fnnas.com>

md: suspend array when sync_action=reshape

This patch modifies action_store() to suspend and lock the array
before starting a reshape operation, and resume it upon exit. This
prevents new requests from allocating objects from an old pool while
the array is being reconfigured.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd5..1377c407614c6 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5263,21 +5263,25 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>  	if (!mddev->pers || !mddev->pers->sync_request)
>  		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does checking mddev->pers locklessly
here risk a NULL pointer dereference later in the function?

A process writing "reshape" to sync_action locklessly verifies
mddev->pers != NULL here. If it is preempted or blocks in flush_work(),
a concurrent write to array_state could stop the array, taking
reconfig_mutex and calling do_md_stop(), which sets mddev->pers = NULL.

When the first process resumes, it acquires reconfig_mutex via
mddev_lock() and calls mddev_start_reshape() without re-verifying
mddev->pers:

mddev_start_reshape() {
    if (mddev->pers->start_reshape == NULL)
    ...
}

Could this cause a kernel panic if do_md_stop() has cleared mddev->pers
concurrently?

>  
> +	action = md_sync_action_by_name(page);
>  retry:
>  	if (work_busy(&mddev->sync_work))
>  		flush_work(&mddev->sync_work);
>  
> -	ret = mddev_lock(mddev);
> +	ret = (action == ACTION_RESHAPE) ?
> +		mddev_suspend_and_lock(mddev) :
> +		mddev_lock(mddev);

[Severity: Critical]
This isn't a bug introduced by this patch, but does exposing
mddev_suspend_and_lock() to concurrent sysfs writes via ACTION_RESHAPE
risk corrupting current->flags?

If two processes concurrently write "reshape" to the sync_action sysfs
file, they both execute mddev_suspend_and_lock() -> mddev_suspend()
concurrently without external serialization.

The first process increments mddev->suspended from 0 to 1 and saves
its task-local NOIO state into the shared mddev->noio_flag:

mddev_suspend() {
    ...
    if (!mddev->suspended++) {
        ...
        mddev->noio_flag = memalloc_noio_save();
    ...
}

The second process increments it from 1 to 2 and skips saving its state.

Upon completion, the first process calls mddev_resume(), decrements the
count to 1, and returns without restoring its state. The second process
calls mddev_resume(), decrements the count to 0, and restores the first
process's state onto itself:

__mddev_resume() {
    ...
    if (--mddev->suspended)
        return;
    ...
    memalloc_noio_restore(mddev->noio_flag);
}

Can this cause a permanent leak of the PF_MEMALLOC_NOIO flag for the
first thread and incorrectly apply it to the last thread?

[Severity: High]
This is also a pre-existing issue in mddev_suspend(), but does calling it
here before acquiring reconfig_mutex allow an unprotected access to
mddev->pers?

If a process writes "reshape", it invokes mddev_suspend() which evaluates
mddev->pers without holding reconfig_mutex or mddev->lock:

mddev_suspend() {
    ...
    if (mddev->pers && mddev->pers->prepare_suspend &&
        reshape_interrupted(mddev))
        mddev->pers->prepare_suspend(mddev);
    ...
}

Concurrently, another process calling do_md_stop() could set
mddev->pers = NULL and drop the personality module reference. If
mddev_suspend() checks mddev->pers, but the module is unloaded before
calling mddev->pers->prepare_suspend(), could this result in a NULL
pointer dereference or execution of unmapped memory?

>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623123840.2521340-1-chencheng@fnnas.com?part=1

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

* Re: [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape
  2026-06-23 12:38 ` [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
@ 2026-06-23 13:00   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-23 13:00 UTC (permalink / raw)
  To: Chen Cheng; +Cc: yukuai, linux-raid

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly allocated `new_pool` is leaked on the `reshape_backwards` error path.
--

commit 2142096170ffdd8a1caabfde6f2461f6e7d64f61
Author: Chen Cheng <chencheng@fnnas.com>

md/raid10: resize r10bio_pool for reshape
    
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.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index cee5a253a281d..d740744a97461 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -4411,6 +4422,11 @@ static int raid10_start_reshape(struct mddev *mddev)
>  		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,

[Severity: Medium]
Will this newly allocated new_pool be leaked if the reshape_backwards
error path is triggered?

If an array is configured for a reshape that expands the number of disks
(delta_disks > 0), the new_pool is allocated here. However, if the
component device sizes are also reduced such that the array is set to
reshape backwards, it can hit a pre-existing error check a few lines
down:

	if (mddev->reshape_backwards) {
		sector_t size = raid10_size(mddev, 0, 0);
		if (size < mddev->array_sectors) {
			spin_unlock_irq(&conf->device_lock);
			pr_warn("md/raid10:%s: array size must be reduce before number of disks\n",
				mdname(mddev));
			return -EINVAL;
		}

This direct return skips the standard cleanup path at the abort label,
causing the mempool_t structure and its preallocated elements to leak.
Should we add a mempool_destroy(new_pool) here, or change the return to
a goto abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623123840.2521340-1-chencheng@fnnas.com?part=2

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

end of thread, other threads:[~2026-06-23 13:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-23 12:38 [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape Chen Cheng
2026-06-23 12:38 ` [PATCH v6 1/3] md: suspend array when sync_action=reshape Chen Cheng
2026-06-23 12:55   ` sashiko-bot
2026-06-23 12:38 ` [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape Chen Cheng
2026-06-23 13:00   ` sashiko-bot
2026-06-23 12:38 ` [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio() Chen Cheng

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