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

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

[Rosen Penev]

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.

Assisted-by: LLM
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: fix subject
 drivers/dma/ppc4xx/adma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
index 1bdc781efbb5..39498779e241 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);
 	BUG();
 	return slot_cnt;
@@ -1989,7 +1989,7 @@ static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_memcpy(
 	spin_lock_bh(&ppc440spe_chan->lock);
 
 	dev_dbg(ppc440spe_chan->device->common.dev,
-		"ppc440spe adma%d: %s len: %u int_en %d\n",
+		"ppc440spe adma%d: %s len: %zu int_en %d\n",
 		ppc440spe_chan->device->id, __func__, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 	slot_cnt = slots_per_op = 1;
@@ -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",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 
@@ -2279,7 +2279,7 @@ static struct ppc440spe_adma_desc_slot *ppc440spe_dma01_prep_pq(
 	unsigned long op = 0;
 	unsigned char mult = 1;
 
-	pr_debug("%s: dst_cnt %d, src_cnt %d, len %d\n",
+	pr_debug("%s: dst_cnt %d, src_cnt %d, len %zu\n",
 		 __func__, dst_cnt, src_cnt, len);
 	/*  select operations WXOR/RXOR depending on the
 	 * source addresses of operators and the number
@@ -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",
 		ppc440spe_chan->device->id, __func__, src_cnt, len,
 		flags & DMA_PREP_INTERRUPT ? 1 : 0);
 
@@ -4071,7 +4071,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);
 		initcode = PPC_ADMA_INIT_COHERENT;
-- 
2.55.0

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

[sashiko-bot]

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