[PATCH 1/2] md/raid5: fix pool_size leak in resize_stripes error path

[PATCH 1/2] md/raid5: fix pool_size leak in resize_stripes error path

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In resize_stripes(), conf->disks is replaced with a new array (ndisks)
in Step 3, but pool_size is only updated at the end with
"if (!err) conf->pool_size = newsize".  If Step 4 (allocating pages for
new stripes) fails, err is set but pool_size is not updated, even though
conf->disks already has newsize entries.

When the array is later torn down, free_conf() iterates only pool_size
(= old value) entries, leaking (newsize - pool_size) extra_page
allocations.

Set pool_size immediately after replacing conf->disks, so it always
matches the actual array size regardless of subsequent failures.

Fixes: ad01c9e3752f ("[PATCH] md: Allow stripes to be expanded in preparation for expanding an array")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
 drivers/md/raid5.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2641,6 +2641,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 		} else {
 			kfree(conf->disks);
 			conf->disks = ndisks;
+			conf->pool_size = newsize;
 		}
 	} else
 		err = -ENOMEM;
@@ -2687,5 +2688,3 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 	/* critical section pass, GFP_NOIO no longer needed */
 
-	if (!err)
-		conf->pool_size = newsize;
 	mutex_unlock(&conf->cache_size_mutex);
-- 
2.34.1

[PATCH 2/2] md/raid5: fix NULL pointer dereference in raid5_free_percpu

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

If cpuhp_state_add_instance() fails in raid5_alloc_percpu() (e.g., the
startup callback raid456_cpu_up_prepare fails due to an allocation
failure), conf->node is never added to the cpuhp instance list and
its pprev remains NULL (from kzalloc initialization).

When setup_conf() then jumps to abort, free_conf() calls
raid5_free_percpu(), which checks conf->percpu (non-NULL, since it was
allocated before the cpuhp failure) and proceeds to call
cpuhp_state_remove_instance().  This calls hlist_del() on the unhashed
node, which dereferences node->pprev (NULL), causing a kernel panic.

Guard the removal with hlist_unhashed() so that the cpuhp instance is
only removed if it was actually added.

Fixes: 29c6d1bbd7a2 ("md/raid5: Convert to hotplug state machine")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
 drivers/md/raid5.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7539,7 +7539,8 @@ static void raid5_free_percpu(struct r5conf *conf)
 {
 	if (!conf->percpu)
 		return;
 
-	cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
+	if (!hlist_unhashed(&conf->node))
+		cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
 	free_percpu(conf->percpu);
 }
-- 
2.34.1

Re: [PATCH 1/2] md/raid5: fix pool_size leak in resize_stripes error path

[sashiko-bot]

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

New issues:
- [High] Premature update of `pool_size` causes subsequent reshape retries to bypass stripe cache resizing, leading to a NULL pointer dereference.

Pre-existing issues:
- [High] Lockless access to `conf->disks` in `raid5_status()` can lead to a Use-After-Free if it races with `resize_stripes()` freeing the array.
--

commit 021dd13adb2ffa7a0e256f63ee764cd6ee6f46ab
Author: Cao Guanghui <caoguanghui@kylinos.cn>

md/raid5: fix pool_size leak in resize_stripes error path

This commit updates conf->pool_size immediately after replacing conf->disks
in resize_stripes(). It intends to ensure pool_size matches the allocated
array size so free_conf() frees the correct number of extra page allocations
on teardown.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090de..fa64cf059b8ea 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -2642,6 +2642,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)
>  		} else {
>  			kfree(conf->disks);

[Severity: High]
This is a pre-existing issue, but does freeing the old disks array here
without synchronization (e.g., an RCU grace period) allow a use-after-free?

Concurrent readers like raid5_status() access the freed disks array locklessly
since md_seq_show() only holds mddev->lock (a spinlock), while this code
executes under mddev->reconfig_mutex and conf->cache_size_mutex.

For example, in raid5_status():

drivers/md/raid5.c:raid5_status() {
    ...
    struct md_rdev *rdev = READ_ONCE(conf->disks[i].rdev);
    ...
}

If an unprivileged user reads /proc/mdstat exactly when root initiates a
reshape, it appears the kernel will dereference the memory freed here.

>  			conf->disks = ndisks;
> +			conf->pool_size = newsize;

[Severity: High]
Does moving the pool_size update here cause a NULL pointer dereference on
reshape retries?

If the alloc_page() calls in Step 4 of resize_stripes() fail due to memory
pressure, the function sets err = -ENOMEM and returns, but conf->pool_size
has already been updated to newsize. The stripes are then released into the
cache with missing (NULL) pages.

If a user retries the array reshape operation after the -ENOMEM failure,
check_reshape() will evaluate this condition:

drivers/md/raid5.c:check_reshape() {
    ...
    if (conf->previous_raid_disks + mddev->delta_disks <= conf->pool_size)
        return 0; /* never bother to shrink */
    ...
}

This evaluates to true because pool_size was already updated. The retry
bypasses resize_stripes() and proceeds with the reshape. Later, when
operations like ops_run_io() access the unallocated pages for the new
devices, it results in a NULL pointer dereference.

>  		}
>  	} else
>  		err = -ENOMEM;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827031646.49080-1-ghuicao@163.com?part=1