[PATCH v2] dmaengine: pxa: fix double counting of the hw descriptors

[PATCH v2] dmaengine: pxa: fix double counting of the hw descriptors

[Sascha Hauer]

pxad_alloc_desc() was converted from

        kzalloc(struct_size(sw_desc, hw_desc, nb_hw_desc), GFP_NOWAIT)

to kzalloc_flex(), which sets the __counted_by() counter sw_desc->nb_desc
itself - but only where the compiler has __builtin_counted_by_ref(), so
from gcc 15.1 or clang 22.1 on. The loop below it still increments
nb_desc, which makes it come out doubled there and correct elsewhere.

nb_desc is what pxad_free_desc() iterates over and what
set_updater_desc() indexes from, so set it explicitly and drop the
increment. The error path has to lower it to the number of descriptors
allocated so far, otherwise pxad_free_desc() would free entries that were
never allocated.

Fixes: 69050f8d6d075 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
pxad_alloc_desc() was converted from

        kzalloc(struct_size(sw_desc, hw_desc, nb_hw_desc), GFP_NOWAIT)

to kzalloc_flex(). hw_desc[] is annotated with __counted_by(nb_desc), so
__alloc_flex() now initializes sw_desc->nb_desc to nb_hw_desc itself.
The loop below it still increments nb_desc for every descriptor it
allocates though, so nb_desc ends up being twice the number of
descriptors that are actually there.

Drop the now redundant increment. The error path has to set nb_desc to
the number of descriptors allocated so far, otherwise pxad_free_desc()
would free entries that were never allocated.
---
Changes in v2:
- assign sw_desc->nb_desc manually as it is only automatically assigned
  for compilers supporting __builtin_counted_by_ref()
- Link to v1: https://lore.kernel.org/r/20260817-dmaengine-pxa-v1-1-850c215c1196@pengutronix.de
---
 drivers/dma/pxa_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index fa2ee0b3e09f8..fc43124fefa89 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -744,6 +744,7 @@ pxad_alloc_desc(struct pxad_chan *chan, unsigned int nb_hw_desc)
 	sw_desc = kzalloc_flex(*sw_desc, hw_desc, nb_hw_desc, GFP_NOWAIT);
 	if (!sw_desc)
 		return NULL;
+	sw_desc->nb_desc = nb_hw_desc;
 	sw_desc->desc_pool = chan->desc_pool;
 
 	for (i = 0; i < nb_hw_desc; i++) {
@@ -752,10 +753,10 @@ pxad_alloc_desc(struct pxad_chan *chan, unsigned int nb_hw_desc)
 			dev_err(&chan->vc.chan.dev->device,
 				"%s(): Couldn't allocate the %dth hw_desc from dma_pool %p\n",
 				__func__, i, sw_desc->desc_pool);
+			sw_desc->nb_desc = i;
 			goto err;
 		}
 
-		sw_desc->nb_desc++;
 		sw_desc->hw_desc[i] = desc;
 
 		if (i == 0)

---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260817-dmaengine-pxa-64152bb34313

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH v2] dmaengine: pxa: fix double counting of the hw descriptors

[sashiko-bot]

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

Pre-existing issues:
- [High] For cyclic DMA transfers, the updater descriptor's DMA address is leaked, and the first descriptor's DMA address is double-freed in the DMA pool.
--

--- Patch [1]: [PATCH v2] dmaengine: pxa: fix double counting of the hw descriptors ---
commit 5e37421334e2f54f86d40ee190a04a05eb99596b
Author: Sascha Hauer <s.hauer@pengutronix.de>

dmaengine: pxa: fix double counting of the hw descriptors

This commit fixes a double counting issue with sw_desc->nb_desc in
pxad_alloc_desc() caused by the transition to kzalloc_flex(). It explicitly
sets nb_desc and removes the redundant increment in the allocation loop.

Link: https://lore.kernel.org/r/20260817-dmaengine-pxa-v1-1-850c215c1196@pengutronix.de

> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index fa2ee0b3e09f8..fc43124fefa89 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c

[ ... ]

> @@ -744,6 +744,7 @@ pxad_alloc_desc(struct pxad_chan *chan, unsigned int nb_hw_desc)
>  	sw_desc = kzalloc_flex(*sw_desc, hw_desc, nb_hw_desc, GFP_NOWAIT);
>  	if (!sw_desc)
>  		return NULL;
> +	sw_desc->nb_desc = nb_hw_desc;

[Severity: High]
This isn't a bug introduced by this patch, but since this patch corrects the
nb_desc tracking used by set_updater_desc() and pxad_free_desc(), is there a
pre-existing double-free for cyclic transfers?

When a cyclic transfer is set up, set_updater_desc() creates a hardware loop
by overwriting the ddadr of the second-to-last descriptor:

drivers/dma/pxa_dma.c:set_updater_desc() {
    ...
    if (sw_desc->cyclic)
        sw_desc->hw_desc[sw_desc->nb_desc - 2]->ddadr = sw_desc->first;
    ...
}

Later, pxad_free_desc() relies on this overwritten field to retrieve the DMA
address to free the next descriptor in the chain:

drivers/dma/pxa_dma.c:pxad_free_desc() {
    ...
    for (i = sw_desc->nb_desc - 1; i >= 0; i--) {
        if (i > 0)
            dma = sw_desc->hw_desc[i - 1]->ddadr;
        else
            dma = sw_desc->first;
        dma_pool_free(sw_desc->desc_pool,
                      sw_desc->hw_desc[i], dma);
    }
    ...
}

Because ddadr was overwritten with sw_desc->first, doesn't pxad_free_desc()
read sw_desc->first instead of the original DMA address when freeing the
updater descriptor?

Could this lead to double-freeing the first descriptor's DMA address and
permanently leaking the updater descriptor's original address in the DMA pool?

>  	sw_desc->desc_pool = chan->desc_pool;
>  
>  	for (i = 0; i < nb_hw_desc; i++) {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-dmaengine-pxa-v2-1-f42ab0569a48@pengutronix.de?part=1

Re: [PATCH v2] dmaengine: pxa: fix double counting of the hw descriptors

[Frank Li]

On Mon, Aug 17, 2026 at 10:44:33PM +0200, Sascha Hauer wrote:
> pxad_alloc_desc() was converted from
>
>         kzalloc(struct_size(sw_desc, hw_desc, nb_hw_desc), GFP_NOWAIT)
>
> to kzalloc_flex(), which sets the __counted_by() counter sw_desc->nb_desc
> itself - but only where the compiler has __builtin_counted_by_ref(), so
> from gcc 15.1 or clang 22.1 on. The loop below it still increments
> nb_desc, which makes it come out doubled there and correct elsewhere.
>
> nb_desc is what pxad_free_desc() iterates over and what
> set_updater_desc() indexes from, so set it explicitly and drop the
> increment. The error path has to lower it to the number of descriptors
> allocated so far, otherwise pxad_free_desc() would free entries that were
> never allocated.
>
> Fixes: 69050f8d6d075 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> pxad_alloc_desc() was converted from
>
>         kzalloc(struct_size(sw_desc, hw_desc, nb_hw_desc), GFP_NOWAIT)
>
> to kzalloc_flex(). hw_desc[] is annotated with __counted_by(nb_desc), so
> __alloc_flex() now initializes sw_desc->nb_desc to nb_hw_desc itself.
> The loop below it still increments nb_desc for every descriptor it
> allocates though, so nb_desc ends up being twice the number of
> descriptors that are actually there.
>
> Drop the now redundant increment. The error path has to set nb_desc to
> the number of descriptors allocated so far, otherwise pxad_free_desc()
> would free entries that were never allocated.
> ---
> Changes in v2:
> - assign sw_desc->nb_desc manually as it is only automatically assigned
>   for compilers supporting __builtin_counted_by_ref()
> - Link to v1: https://lore.kernel.org/r/20260817-dmaengine-pxa-v1-1-850c215c1196@pengutronix.de
> ---
>  drivers/dma/pxa_dma.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index fa2ee0b3e09f8..fc43124fefa89 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -744,6 +744,7 @@ pxad_alloc_desc(struct pxad_chan *chan, unsigned int nb_hw_desc)
>  	sw_desc = kzalloc_flex(*sw_desc, hw_desc, nb_hw_desc, GFP_NOWAIT);
>  	if (!sw_desc)
>  		return NULL;
> +	sw_desc->nb_desc = nb_hw_desc;
>  	sw_desc->desc_pool = chan->desc_pool;
>
>  	for (i = 0; i < nb_hw_desc; i++) {
> @@ -752,10 +753,10 @@ pxad_alloc_desc(struct pxad_chan *chan, unsigned int nb_hw_desc)
>  			dev_err(&chan->vc.chan.dev->device,
>  				"%s(): Couldn't allocate the %dth hw_desc from dma_pool %p\n",
>  				__func__, i, sw_desc->desc_pool);
> +			sw_desc->nb_desc = i;
>  			goto err;
>  		}
>
> -		sw_desc->nb_desc++;
>  		sw_desc->hw_desc[i] = desc;
>
>  		if (i == 0)
>
> ---
> base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
> change-id: 20260817-dmaengine-pxa-64152bb34313
>
> Best regards,
> --
> Sascha Hauer <s.hauer@pengutronix.de>
>