Re: [PATCH] md/raid5: Cancel remove instance if adding it fails

[yu kuai]

Hi,

在 2026/10/30 18:56, Edward Adam Davis 写道:
> The failure of raid456_cpu_up_prepare() to allocate memory causes
> cpuhp_state_add_instance() to fail; consequently, the corresponding
> instance is not initialized, potentially hitting [1].
>
> Additionally, there is another null-ptr-deref issue: conf->mddev is
> assigned too late, meaning that any error-handling exit occurring before
> this assignment, which jumps to the abort label, potentially hitting [2].
>
> [1]
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> Call Trace:
>   cpuhp_state_remove_instance include/linux/cpuhotplug.h:480 [inline]
>   raid5_free_percpu drivers/md/raid5.c:7541 [inline]
>   free_conf+0x238/0x710 drivers/md/raid5.c:7554
>   setup_conf+0xd80/0x2a20 drivers/md/raid5.c:7885
>   raid5_run+0x1260/0x1670 drivers/md/raid5.c:8129
>
> [2]
> BUG: KASAN: null-ptr-deref in free_conf+0xfd/0x7e0
> Read of size 8 at addr 0000000000000028 by task rp0/9459
> Call Trace:
>   free_conf+0xfd/0x7e0
>   setup_conf.cold+0x6e/0x4a1
>   raid5_run+0x1260/0x1670
>
> Add a check before removing an instance, if the instance was not successfully
> added, skip the removal process. Move the assignment of conf->mddev after the
> memory allocation for conf.
>
> Fixes: 29c6d1bbd7a2 ("md/raid5: Convert to hotplug state machine")
> Reported-by: syzbot+08318698e045b4b85f51@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=08318698e045b4b85f51
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxxxx>
> ---
>   drivers/md/raid5.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Looks like this problem is already fixed

https://patch.msgid.link/20260827080341.112296-2-ghuicao@xxxxxxx
https://patch.msgid.link/20260915081731.122933-1-chengzhihao1@xxxxxxxxxx

>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090d..e1f9ec8f25be 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7538,7 +7538,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);
>   }
>   
> @@ -7675,6 +7676,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>   	if (conf == NULL)
>   		goto abort;
>   
> +	conf->mddev = mddev;
>   #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
>   	conf->stripe_size = DEFAULT_STRIPE_SIZE;
>   	conf->stripe_shift = ilog2(DEFAULT_STRIPE_SIZE) - 9;
> @@ -7743,7 +7745,6 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>   	ret = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
>   	if (ret)
>   		goto abort;
> -	conf->mddev = mddev;
>   
>   	ret = -ENOMEM;
>   	conf->stripe_hashtbl = kzalloc(PAGE_SIZE, GFP_KERNEL);

-- 
Thanks,
Kuai