Re: [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path

[yu kuai]

Hi,

在 2026/10/4 14:02, Yogesh Gaur 写道:
> free_conf() starts with log_exit(), which falls through to
> raid5_has_ppl() when conf->log is NULL:
>
> 	static inline void log_exit(struct r5conf *conf)
> 	{
> 		if (conf->log)
> 			r5l_exit_log(conf);
> 		else if (raid5_has_ppl(conf))
> 			ppl_exit_log(conf);
> 	}
>
> raid5_has_ppl() reads conf->mddev->flags. setup_conf() only assigns
> conf->mddev after bioset_init(), but five of its error paths -- the
> pending_data, alloc_thread_groups(), conf->disks, extra_page and
> bioset_init() failures -- jump to "abort:" before that, and abort: calls
> free_conf(). conf comes from kzalloc, so conf->mddev is still NULL and
> free_conf() dereferences it:
>
> 	BUG: KASAN: null-ptr-deref in raid5_has_ppl drivers/md/raid5-log.h:54 [inline]
> 	BUG: KASAN: null-ptr-deref in log_exit drivers/md/raid5-log.h:128 [inline]
> 	BUG: KASAN: null-ptr-deref in free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> 	Read of size 8 at addr 0000000000000028 by task syz.3.20/5681
> 	Call Trace:
> 	 <TASK>
> 	 free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> 	 setup_conf+0x1720/0x2ad0 drivers/md/raid5.c:7885
> 	 raid5_run+0x8cc/0x2560 drivers/md/raid5.c:8129
> 	 md_run+0xc3d/0x1cd0 drivers/md/md.c:6779
> 	 do_md_run+0x35/0x720 drivers/md/md.c:6880
> 	 array_state_store+0x958/0xe90 drivers/md/md.c:-1
> 	 </TASK>
>
> The faulting address is offsetof(struct mddev, flags).
>
> conf->mddev is a back pointer that is constant for the lifetime of the
> conf and does not depend on anything computed in between, so assign it
> as soon as the conf is allocated. Nothing between the allocation and the
> old assignment reads conf->mddev -- alloc_thread_groups() takes the conf
> but never looks at its mddev, and the rdev_for_each() loop walks the
> mddev argument directly.
>
> All five paths are allocation failures, so this needs memory pressure or
> fault injection to hit, which is how syzbot found it.
>
> Reported-by: syzbot+270624bb31d478afe62e@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=270624bb31d478afe62e
> Fixes: ff875738edd4 ("raid5: separate header for log functions")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@xxxxxxxxx>
> ---
>   drivers/md/raid5.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090d..f98d3bdd3484 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7675,6 +7675,9 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>   	if (conf == NULL)
>   		goto abort;
>   
> +	/* free_conf() dereferences this, so set it before the first goto */
> +	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 +7746,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);

The same patch is already applied.

https://patch.msgid.link/20260915081731.122933-1-chengzhihao1@xxxxxxxxxx

-- 
Thanks,
Kuai