dmaengine: ppc4xx: check dma_map_page() errors in probe

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

Posted by Rosen Penev 2 weeks ago

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

Posted by Frank Li 1 week, 6 days ago

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

Posted by Rosen Penev 1 week, 6 days ago

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
>>