Re: [PATCH] dmaengine: bestcomm: ioremap early

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

Pre-existing issues:
- [High] The globally exported pointer `bcom_eng` is left dangling if `mpc52xx_bcom_probe()` fails after allocating it, leading to a potential Use-After-Free.
--

--- Patch [1]: [PATCH] dmaengine: bestcomm: ioremap early ---
commit 58088fd211b9916066bb25d1ffe4afa62733063f
Author: Rosen Penev <[email protected]>

dmaengine: bestcomm: ioremap early

This commit replaces open-coded resource lookup and mapping in the
probe function with the managed devm_platform_get_and_ioremap_resource()
helper. It also addresses a resource size mismatch between allocation
and freeing.

> diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
> --- a/drivers/dma/bestcomm/bestcomm.c
> +++ b/drivers/dma/bestcomm/bestcomm.c

[ ... ]

> @@ -439,10 +422,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>  	return 0;
>  
>  	/* Error path */
> -error_unmap:
> -	iounmap(bcom_eng->regs);
> -error_release:
> -	release_mem_region(res_bcom.start, sizeof(struct mpc52xx_sdma));
>  error_sramclean:
>  	kfree(bcom_eng);

[Severity: High]
This is a pre-existing issue, but if mpc52xx_bcom_probe() fails after
allocating bcom_eng (for instance, if bcom_engine_init() fails), does this
leave the globally exported pointer bcom_eng dangling?

If it remains non-NULL after kfree(), a dependent driver like mpc52xx_fec
might later call bcom_task_alloc(), which checks if (!bcom_eng) to verify
initialization. Since the check passes, could this lead to a use-after-free
when it accesses bcom_eng->lock?

Should bcom_eng be set to NULL here, similar to how it is handled in
mpc52xx_bcom_remove()?

>  	bcom_sram_cleanup();

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