[PATCH] dmaengine: xilinx: Fix wild pointer dereference on empty segment list

[PATCH] dmaengine: xilinx: Fix wild pointer dereference on empty segment list

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

list_first_entry() does not return NULL when the list is empty; it
returns the result of container_of() on the list head, i.e. a wild
pointer that points into the enclosing xilinx_dma_tx_descriptor rather
than to a valid segment. Dereferencing it reads or writes unrelated
descriptor memory. This was found by code audit.

In xilinx_dma_prep_peripheral_dma_vec(), xilinx_dma_prep_slave_sg()
and xilinx_mcdma_prep_slave_sg(), the loop that populates
desc->segments never executes when the transfer length is zero
(nb == 0, sg_len == 0, or all vecs[i].len / sg_dma_len() entries are
zero). The dmaengine inline wrappers do not reject such zero-length
transfers, so they reach the driver unchanged. The unconditional
list_first_entry() then yields a wild pointer: the following
head->phys / segment->phys reads are out-of-bounds reads within the
descriptor slab, and on the DMA_MEM_TO_DEV path the hw.control
read-modify-write corrupts the descriptor. In
xilinx_dma_prep_peripheral_dma_vec() the empty case additionally
dereferences the never-initialized 'segment' pointer. Similarly, in
xilinx_dma_get_metadata_ptr(), a descriptor with an empty segment
list produces a wild 'seg' whose hw.app pointer is handed back to the
DMA client.

Guard each list_first_entry() call: in the three prep functions, bail
out through the existing error path (freeing the descriptor and
returning NULL, consistent with the current failure handling) when
desc->segments is still empty after the build loop; in
xilinx_dma_get_metadata_ptr(), use list_first_entry_or_null() and
return NULL when there is no segment.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index cffe7c6fa..07f52f841 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -650,8 +650,11 @@ static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
 	struct xilinx_axidma_tx_segment *seg;
 
 	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
-	seg = list_first_entry(&desc->segments,
-			       struct xilinx_axidma_tx_segment, node);
+	seg = list_first_entry_or_null(&desc->segments,
+				       struct xilinx_axidma_tx_segment, node);
+	if (!seg)
+		return NULL;
+
 	return seg->hw.app;
 }
 
@@ -2305,6 +2308,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	head = list_first_entry(&desc->segments, struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = head->phys;
 
@@ -2406,6 +2412,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;
@@ -2613,6 +2622,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_aximcdma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;
-- 
2.30.2

Re: [PATCH] dmaengine: xilinx: Fix wild pointer dereference on empty segment list

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: xilinx: NULL return in xilinx_dma_get_metadata_ptr()
--

commit 5ff31a1fefdaf467f495be421ba38281e2cbd1a6
Author: Wang Haoran <haoranwangsec@gmail.com>

dmaengine: xilinx: Fix wild pointer dereference on empty segment list

This commit guards against empty segment lists in the Xilinx DMA prep
functions and in xilinx_dma_get_metadata_ptr() to avoid dereferencing wild
pointers from list_first_entry().

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index cffe7c6fa6405..07f52f8412b10 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -650,8 +650,11 @@ static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
>  	struct xilinx_axidma_tx_segment *seg;
>  
>  	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> -	seg = list_first_entry(&desc->segments,
> -			       struct xilinx_axidma_tx_segment, node);
> +	seg = list_first_entry_or_null(&desc->segments,
> +				       struct xilinx_axidma_tx_segment, node);
> +	if (!seg)
> +		return NULL;

[Severity: High]
Should xilinx_dma_get_metadata_ptr() return an ERR_PTR() (such as
ERR_PTR(-EINVAL) or ERR_PTR(-ENODATA)) before modifying *max_len and
*payload_len when desc->segments is empty, rather than returning NULL?

The caller in drivers/dma/dmaengine.c uses the ERR_PTR() error convention and
directly returns the result of metadata_ops->get_ptr():

void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
				      size_t *payload_len, size_t *max_len)
{
	int ret;

	if (!desc)
		return ERR_PTR(-EINVAL);

	ret = desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
	if (ret)
		return ERR_PTR(ret);

	if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
		return ERR_PTR(-ENOTSUPP);

	return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
}

Callers of dmaengine_desc_get_metadata_ptr(), such as axienet_dma_rx_cb() in
drivers/net/ethernet/xilinx/xilinx_axienet_main.c, check the returned pointer
with IS_ERR() before dereferencing it:

	app_metadata = dmaengine_desc_get_metadata_ptr(skbuf_dma->desc, &meta_len,
						       &meta_max_len);
	...
	if (IS_ERR(app_metadata)) {
		if (net_ratelimit())
			netdev_err(lp->ndev, "Failed to get RX metadata pointer\n");
		dev_kfree_skb_any(skb);
		lp->ndev->stats.rx_dropped++;
		goto rx_submit;
	}

	/* TODO: Derive app word index programmatically */
	rx_len = (app_metadata[LEN_APP] & 0xFFFF);

Because IS_ERR(NULL) is false, returning NULL when !seg bypasses the caller's
error check and leads to a NULL pointer dereference, while also mutating
*max_len and *payload_len on failure.

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

Re: [PATCH] dmaengine: xilinx: Fix wild pointer dereference on empty segment list

[Frank Li]

On Fri, Oct 23, 2026 at 07:10:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] dmaengine: xilinx: NULL return in xilinx_dma_get_metadata_ptr()
> --
>
> commit 5ff31a1fefdaf467f495be421ba38281e2cbd1a6
> Author: Wang Haoran <haoranwangsec@gmail.com>
>
> dmaengine: xilinx: Fix wild pointer dereference on empty segment list
>
> This commit guards against empty segment lists in the Xilinx DMA prep
> functions and in xilinx_dma_get_metadata_ptr() to avoid dereferencing wild
> pointers from list_first_entry().
>
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > index cffe7c6fa6405..07f52f8412b10 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -650,8 +650,11 @@ static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
> >  	struct xilinx_axidma_tx_segment *seg;
> >
> >  	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> > -	seg = list_first_entry(&desc->segments,
> > -			       struct xilinx_axidma_tx_segment, node);
> > +	seg = list_first_entry_or_null(&desc->segments,
> > +				       struct xilinx_axidma_tx_segment, node);
> > +	if (!seg)
> > +		return NULL;
>
> [Severity: High]
> Should xilinx_dma_get_metadata_ptr() return an ERR_PTR() (such as
> ERR_PTR(-EINVAL) or ERR_PTR(-ENODATA)) before modifying *max_len and
> *payload_len when desc->segments is empty, rather than returning NULL?
>
> The caller in drivers/dma/dmaengine.c uses the ERR_PTR() error convention and
> directly returns the result of metadata_ops->get_ptr():
>
> void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
> 				      size_t *payload_len, size_t *max_len)
> {
> 	int ret;
>
> 	if (!desc)
> 		return ERR_PTR(-EINVAL);
>
> 	ret = desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> 	if (ret)
> 		return ERR_PTR(ret);
>
> 	if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
> 		return ERR_PTR(-ENOTSUPP);
>
> 	return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
> }
>
> Callers of dmaengine_desc_get_metadata_ptr(), such as axienet_dma_rx_cb() in
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c, check the returned pointer
> with IS_ERR() before dereferencing it:
>
> 	app_metadata = dmaengine_desc_get_metadata_ptr(skbuf_dma->desc, &meta_len,
> 						       &meta_max_len);
> 	...
> 	if (IS_ERR(app_metadata)) {
> 		if (net_ratelimit())
> 			netdev_err(lp->ndev, "Failed to get RX metadata pointer\n");
> 		dev_kfree_skb_any(skb);
> 		lp->ndev->stats.rx_dropped++;
> 		goto rx_submit;
> 	}
>
> 	/* TODO: Derive app word index programmatically */
> 	rx_len = (app_metadata[LEN_APP] & 0xFFFF);
>
> Because IS_ERR(NULL) is false, returning NULL when !seg bypasses the caller's
> error check and leads to a NULL pointer dereference, while also mutating
> *max_len and *payload_len on failure.

Make sense, please fix it.

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