Re: [PATCH v2 1/3] md/raid5: set pool_size before extra_page allocation to fix leak on error path

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

Pre-existing issues:
- [High] NULL pointer dereference in log_exit() when early allocations in setup_conf() fail before conf->mddev is initialized.
--

commit 9327d7b2c0b735410bac76b4b55b188fa14875d3
Author: Cao Guanghui <[email protected]>

    md/raid5: set pool_size before extra_page allocation to fix leak on error path

    In setup_conf(), conf->disks is allocated with max_disks slots and
    extra_page is allocated for each slot. However, pool_size remains 0
    (uninitialized from kzalloc) until grow_stripes() sets it later. This
    fixes a memory leak by setting pool_size earlier.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090de..4efaef622fc05 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7733,6 +7733,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  
>  	if (!conf->disks)
>  		goto abort;
> +	conf->pool_size = max_disks;
>  
>  	for (i = 0; i < max_disks; i++) {
>  		conf->disks[i].extra_page = alloc_page(GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer
dereference on these early error paths.

If any early memory allocation in setup_conf() fails (such as
conf->pending_data, conf->disks, or alloc_page() for extra pages here),
the code jumps to the abort label and calls free_conf(conf). This happens
before conf->mddev is assigned.

free_conf() unconditionally calls log_exit(conf). Because conf->log is NULL
on these paths, log_exit() checks raid5_has_ppl():

drivers/md/raid5-log.h:log_exit() {
	if (conf->log)
		r5l_exit_log(conf);
	else if (raid5_has_ppl(conf))
		ppl_exit_log(conf);
}

This function evaluates test_bit(MD_HAS_PPL, &conf->mddev->flags), which
dereferences the uninitialized conf->mddev pointer:

drivers/md/raid5-log.h:raid5_has_ppl() {
	return test_bit(MD_HAS_PPL, &conf->mddev->flags);
}

Can conf->mddev = mddev; be assigned immediately after conf is allocated in
setup_conf() to avoid this crash during early aborts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1