[PATCH v2] dmaengine: xilinx: dpdma: fix crash on zero-length cyclic transfer

[PATCH v2] dmaengine: xilinx: dpdma: fix crash on zero-length cyclic transfer

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

When buf_len is zero, the descriptor build loop never runs, leaving the
descriptor list empty. The following list_first_entry() on the empty list
returns a bad pointer and causes a kernel oops.

Return NULL if the descriptor list is empty after the loop.

This issue was found by static analysis.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
v1: https://lore.kernel.org/r/20261023065900.3455414-1-lixicaicai@gmail.com
v2: rewrite the commit message in simpler words as suggested by Frank
Li; no code change.

 drivers/dma/xilinx/xilinx_dpdma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index d9a3542c4..bdfa85b80 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -730,6 +730,9 @@ xilinx_dpdma_chan_prep_cyclic(struct xilinx_dpdma_chan *chan,
 		last = sw_desc;
 	}
 
+	if (list_empty(&tx_desc->descriptors))
+		goto error;
+
 	sw_desc = list_first_entry(&tx_desc->descriptors,
 				   struct xilinx_dpdma_sw_desc, node);
 	last->hw.next_desc = lower_32_bits(sw_desc->dma_addr);
-- 
2.30.2

Re: [PATCH v2] dmaengine: xilinx: dpdma: fix crash on zero-length cyclic transfer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: xilinx: dpdma: misplaced and incomplete zero-length check
--

commit 9ce2097539c963154302835e8e5bdaba12caffe7
Author: Wang Haoran <haoranwangsec@gmail.com>

dmaengine: xilinx: dpdma: fix crash on zero-length cyclic transfer

This commit checks whether the descriptor list is empty after the loop in
xilinx_dpdma_chan_prep_cyclic() to avoid a crash when buf_len is zero.

> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index d9a3542c4531c..bdfa85b80c6a8 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -730,6 +730,9 @@ xilinx_dpdma_chan_prep_cyclic(struct xilinx_dpdma_chan *chan,
>  		last = sw_desc;
>  	}
>  
> +	if (list_empty(&tx_desc->descriptors))
> +		goto error;

[Severity: Medium]
In xilinx_dpdma_chan_prep_cyclic(), every iteration of the loop either
jumps to error or appends a descriptor via list_add_tail():

drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_chan_prep_cyclic() {
	unsigned int periods = buf_len / period_len;
	unsigned int i;

	tx_desc = xilinx_dpdma_chan_alloc_tx_desc(chan);
	if (!tx_desc)
		return NULL;

	for (i = 0; i < periods; i++) {
		...
		list_add_tail(&sw_desc->node, &tx_desc->descriptors);

		buf_addr += period_len;
		last = sw_desc;
	}
}

Because of this, list_empty(&tx_desc->descriptors) can only be true when
periods is 0, so checking after the loop allocates tx_desc with
xilinx_dpdma_chan_alloc_tx_desc() only to immediately free it under the
error label.

Also, in the caller xilinx_dpdma_prep_dma_cyclic(), buf_len % period_len
is evaluated without checking whether period_len is zero:

drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_prep_dma_cyclic() {
	if (direction != DMA_MEM_TO_DEV)
		return NULL;

	if (buf_len % period_len)
		return NULL;

	return xilinx_dpdma_chan_prep_cyclic(chan, buf_addr, buf_len,
					     period_len, flags);
}

If period_len is 0 (for example, when both buf_len and period_len are 0),
won't this still trigger a divide-by-zero crash before reaching
list_empty()?

Would it be cleaner to validate if (!buf_len || !period_len) upfront in
xilinx_dpdma_prep_dma_cyclic(), matching how
xilinx_dpdma_prep_interleaved_dma() checks if (!xt->numf || !xt->sgl[0].size)
before allocating a descriptor?

Additionally, the commit message states:

  The following list_first_entry() on the empty list returns a bad pointer
  and causes a kernel oops.

When the list is empty, list_first_entry() computes an offset from
&tx_desc->descriptors and reads sw_desc->dma_addr relative to tx_desc,
whereas the NULL pointer dereference happens on the next statement
(last->hw.next_desc = ...) because last is initialized to NULL and never
updated when periods is 0. Could the commit message be updated to reflect
this?

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

Re: [PATCH v2] dmaengine: xilinx: dpdma: fix crash on zero-length cyclic transfer

[Frank Li]

On Tue, Nov 03, 2026 at 04:31:32PM +0800, Li Ximo wrote:
> From: Wang Haoran <haoranwangsec@gmail.com>
>
> When buf_len is zero, the descriptor build loop never runs, leaving the
> descriptor list empty. The following list_first_entry() on the empty list
> returns a bad pointer and causes a kernel oops.
>
> Return NULL if the descriptor list is empty after the loop.
>
> This issue was found by static analysis.
>
> Reported-by: Wang Haoran <haoranwangsec@gmail.com>
> Reported-by: Li Ximo <lixicaicai@gmail.com>
> Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
> ---

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

> v1: https://lore.kernel.org/r/20261023065900.3455414-1-lixicaicai@gmail.com
> v2: rewrite the commit message in simpler words as suggested by Frank
> Li; no code change.
>
>  drivers/dma/xilinx/xilinx_dpdma.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index d9a3542c4..bdfa85b80 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -730,6 +730,9 @@ xilinx_dpdma_chan_prep_cyclic(struct xilinx_dpdma_chan *chan,
>  		last = sw_desc;
>  	}
>
> +	if (list_empty(&tx_desc->descriptors))
> +		goto error;
> +
>  	sw_desc = list_first_entry(&tx_desc->descriptors,
>  				   struct xilinx_dpdma_sw_desc, node);
>  	last->hw.next_desc = lower_32_bits(sw_desc->dma_addr);
> --
> 2.30.2