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

[sashiko-bot@kernel.org]

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