Skip to contentSkip to conversation

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

PATCH Applied Rosen Penev 4 participants 4 messages Sep 11 – Oct 5 dmaengine

lore
Share & tools
lore

Rosen Penev

author
#0 lore raw

PATCH v2 1/1 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.

Assisted-by LLM
Signed-off-by Rosen Penev <[email protected]>

---
v2: fix subject

 drivers/dma/ppc4xx/adma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/dma/ppc4xx/adma.c +6 -6
index 1bdc781efbb5..39498779e241 100644
@@ -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;
Signature · 1 line
2.55.0
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

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 <[email protected]>

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.

drivers/dma/ppc4xx/adma.c static int ppc440spe_chan_xor_slot_count(size_t len, int src_cnt, · line 803

Quoted 11 lines
drivers/dma/ppc4xx/adma.c +1 -1
index 279a431ccae38..bcc54d584e2bf 100644
@@ -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?

[ ... ]

drivers/dma/ppc4xx/adma.c static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_xor( · line 2033

Quoted 6 lines
drivers/dma/ppc4xx/adma.c +1 -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",

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

[ ... ]

drivers/dma/ppc4xx/adma.c static struct dma_async_tx_descriptor *ppc440spe_adma_prep_dma_pq( · line 2556

Quoted 6 lines
drivers/dma/ppc4xx/adma.c +1 -1
@@ -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?

[ ... ]

drivers/dma/ppc4xx/adma.c static int ppc440spe_adma_probe(struct platform_device *ofdev) · line 4082

Quoted 8 lines
drivers/dma/ppc4xx/adma.c +1 -1
@@ -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.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Reviewed-by on v2 1/1
#2 lore raw
In reply to Rosen Penev
Quoted 7 lines · 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 <[email protected]>

---

Reviewed-by Frank Li <[email protected]>
Quoted 65 lines

v2: fix subject

 drivers/dma/ppc4xx/adma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/dma/ppc4xx/adma.c +6 -7
index 1bdc781efbb5..39498779e241 100644
@@ -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

maintainer Applied
#3 lore raw
In reply to Rosen Penev
Quoted 5 lines · 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.

Applied, thanks!

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

      commit: 7b6aa7904aae77275e4e8e8c69ce8826dc301fa8

Best regards,

Signature · 1 line
~Vinod