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

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

[Rosen Penev]

In ppc440spe_adma_probe() the helper pages are mapped with
dma_map_page() but the returned DMA address is never validated with
dma_mapping_error(). On 440SPe the mapping goes through the
SWIOTLB/direct map, which can fail under memory pressure or with an
IOMMU, returning DMA_MAPPING_ERROR. The bogus address would then be
programmed into the CDBs used by the async validation and
mult/sum_product operations, causing the engine to DMA to or from
arbitrary memory and corrupt data.

Fail the probe when either mapping fails, freeing the pages and
unmapping the first page if the second mapping fails.

Fixes: 12458ea06efd7 ("ppc440spe-adma: adds updated ppc440spe adma driver")
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ppc4xx/adma.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
index 279a431ccae3..89e778896d7a 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;
+		}
 		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;
+		}
 	}
 
 	ref = kmalloc_obj(*ref);
-- 
2.55.0

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

[sashiko-bot]

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

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

[Frank Li]

On Thu, Sep 10, 2026 at 02:43:34PM -0700, Rosen Penev wrote:
> In ppc440spe_adma_probe() the helper pages are mapped with
> dma_map_page() but the returned DMA address is never validated with
> dma_mapping_error(). On 440SPe the mapping goes through the
> SWIOTLB/direct map, which can fail under memory pressure or with an
> IOMMU, returning DMA_MAPPING_ERROR. The bogus address would then be
> programmed into the CDBs used by the async validation and
> mult/sum_product operations, causing the engine to DMA to or from
> arbitrary memory and corrupt data.
>
> Fail the probe when either mapping fails, freeing the pages and
> unmapping the first page if the second mapping fails.
>
> Fixes: 12458ea06efd7 ("ppc440spe-adma: adds updated ppc440spe adma driver")
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/ppc4xx/adma.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae3..89e778896d7a 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);

put these to lable out

Frank

> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  		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;
> +		}
>  	}
>
>  	ref = kmalloc_obj(*ref);
> --
> 2.55.0
>

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

[Rosen Penev]

On Fri Sep 11, 2026 at 9:05 AM PDT, Frank Li wrote:
> On Thu, Sep 10, 2026 at 02:43:34PM -0700, Rosen Penev wrote:
>> In ppc440spe_adma_probe() the helper pages are mapped with
>> dma_map_page() but the returned DMA address is never validated with
>> dma_mapping_error(). On 440SPe the mapping goes through the
>> SWIOTLB/direct map, which can fail under memory pressure or with an
>> IOMMU, returning DMA_MAPPING_ERROR. The bogus address would then be
>> programmed into the CDBs used by the async validation and
>> mult/sum_product operations, causing the engine to DMA to or from
>> arbitrary memory and corrupt data.
>>
>> Fail the probe when either mapping fails, freeing the pages and
>> unmapping the first page if the second mapping fails.
>>
>> Fixes: 12458ea06efd7 ("ppc440spe-adma: adds updated ppc440spe adma driver")
>> Assisted-by: opencode:big-pickle
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>>  drivers/dma/ppc4xx/adma.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
>> index 279a431ccae3..89e778896d7a 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);
>
> put these to lable out
Problem there is this is in an if statement. It's not as clean to handle
here directly.
>
> Frank
>
>> +			ret = -ENOMEM;
>> +			goto out;
>> +		}
>>  		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;
>> +		}
>>  	}
>>
>>  	ref = kmalloc_obj(*ref);
>> --
>> 2.55.0
>>