Re: [PATCHv2] dmaengine: ppc4xx: change to %zu format specifiers for size_t arguments

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The macro PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT evaluates to a negative integer, completely bypassing bounds checks on 64-bit platforms due to sign extension.
- [High] Missing dma_mapping_error checks for page mappings in ppc440spe_adma_probe.
--

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

dmaengine: ppc4xx: change to %zu format specifiers for size_t arguments

Several dev_err, dev_dbg, pr_debug, and printk calls use %d or %u format
specifiers for size_t arguments (len, pool_size). Fix them to use %zu to
avoid warnings on 64-bit builds.

> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae38..bcc54d584e2bf 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c
> @@ -799,7 +799,7 @@ static int ppc440spe_chan_xor_slot_count(size_t len, int src_cnt,
>  	if (likely(len <= PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT))
>  		return slot_cnt;
>  
> -	printk(KERN_ERR "%s: len %d > max %d !!\n",
> +	printk(KERN_ERR "%s: len %zu > max %d !!\n",
>  		__func__, len, PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);

[Severity: High]
This is a pre-existing issue, but does this bounds check evaluate correctly on
64-bit platforms?

The macro PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT is defined as (1 << 31), which
evaluates to a negative signed integer. When compared against a 64-bit
unsigned size_t, sign extension causes the macro to be implicitly cast to
0xffffffff80000000UL (or ~18 exabytes).

Since this patch explicitly addresses 64-bit builds by changing to %zu
format specifiers, could this pre-existing issue now allow excessively large
len values to bypass the bounds check and truncate hardware descriptor
configurations?

[ ... ]

> @@ -2030,7 +2030,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_xor(
>  	BUG_ON(len > PPC440SPE_ADMA_XOR_MAX_BYTE_COUNT);
>  
>  	dev_dbg(ppc440spe_chan->device->common.dev,
> -		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
> +		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",

[Severity: High]
This isn't a bug introduced by this patch, but similarly to
ppc440spe_chan_xor_slot_count(), does this BUG_ON() check fail to catch large
len values due to the sign-extended macro definition?

[ ... ]

> @@ -2553,7 +2553,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_pq(
>  	BUG_ON(!dst_cnt);
>  
>  	dev_dbg(ppc440spe_chan->device->common.dev,
> -		"ppc440spe adma%d: %s src_cnt: %d len: %u int_en: %d\n",
> +		"ppc440spe adma%d: %s src_cnt: %d len: %zu int_en: %d\n",

[Severity: High]
This is also a pre-existing issue, but is this another instance where the
length bounds check in BUG_ON() fails due to the sign-extended macro?

[ ... ]

> @@ -4077,7 +4077,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
>  					adev->pool_size, &adev->dma_desc_pool,
>  					GFP_KERNEL);
>  	if (adev->dma_desc_pool_virt == NULL) {
> -		dev_err(&ofdev->dev, "failed to allocate %d bytes of coherent "
> +		dev_err(&ofdev->dev, "failed to allocate %zu bytes of coherent "
>  			"memory for hardware descriptors\n",
>  			adev->pool_size);

[Severity: High]
This isn't a bug introduced by this patch, but further down in this function,
dma_map_page() is called to map chan->pdest and chan->qdest pages:

drivers/dma/ppc4xx/adma.c:ppc440spe_adma_probe() {
...
        chan->pdest = dma_map_page(&ofdev->dev, chan->pdest_page, 0,
                                   PAGE_SIZE, DMA_BIDIRECTIONAL);
        chan->qdest = dma_map_page(&ofdev->dev, chan->qdest_page, 0,
                                   PAGE_SIZE, DMA_BIDIRECTIONAL);
...
}

Are we missing a dma_mapping_error() check for these mappings?

If the mapping fails, it could return an invalid DMA address that gets written
into hardware DMA descriptors, potentially leading to IOMMU faults or memory
corruption.

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