Re: [PATCH] dmaengine: ppc4xx: check dma_map_page() errors in probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The error paths for dma_map_page() failures use `goto out;`, bypassing all resource cleanup and causing a massive memory and resource leak.
--

commit e542c1430356273e51891dbcad4333a7ba32009f
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ppc4xx: check dma_map_page() errors in probe
    
This commit adds validation for the DMA addresses returned by dma_map_page() in
ppc440spe_adma_probe(). It fails the probe and frees the pages when a mapping
error occurs to avoid corrupting data.

> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae38..89e778896d7a5 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c
[ ... ]
> @@ -4156,8 +4156,22 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
>  		}
>  		chan->pdest = dma_map_page(&ofdev->dev, chan->pdest_page, 0,
>  					   PAGE_SIZE, DMA_BIDIRECTIONAL);
> +		if (dma_mapping_error(&ofdev->dev, chan->pdest)) {
> +			__free_page(chan->pdest_page);
> +			__free_page(chan->qdest_page);
> +			ret = -ENOMEM;
> +			goto out;

[Severity: High]
Does jumping directly to out bypass the necessary cleanup for chan, adev, the 
dma_desc_pool, and the ioremap regions?

It looks like earlier errors in ppc440spe_adma_probe() jump to specific error 
labels (such as err_page_alloc) to unwind and free these resources. If the 
pdest mapping fails, jumping directly to out instead of the proper error labels 
appears to leak the channel structure and device resources.

> +		}
>  		chan->qdest = dma_map_page(&ofdev->dev, chan->qdest_page, 0,
>  					   PAGE_SIZE, DMA_BIDIRECTIONAL);
> +		if (dma_mapping_error(&ofdev->dev, chan->qdest)) {
> +			dma_unmap_page(&ofdev->dev, chan->pdest,
> +				       PAGE_SIZE, DMA_BIDIRECTIONAL);
> +			__free_page(chan->pdest_page);
> +			__free_page(chan->qdest_page);
> +			ret = -ENOMEM;
> +			goto out;

[Severity: High]
Similarly, does this goto also leak the chan structure, adev, dma_desc_pool, 
and ioremap regions when the qdest mapping fails? 

Jumping to err_page_alloc might correctly unwind the allocations made earlier 
in the probe function after the pages are freed.

> +		}
>  	}
>  
>  	ref = kmalloc_obj(*ref);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910214334.86689-1-rosenp@gmail.com?part=1