[PATCH net v2] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path

[PATCH net v2] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path

[David Carlier]

On XDP_TX from the zero-copy RX path, emac_run_xdp() converts the xsk
buffer via xdp_convert_zc_to_xdp_frame(), which clones the data into a
fresh MEM_TYPE_PAGE_ORDER0 page that is not DMA mapped. Transmitting it
as PRUETH_TX_BUFF_TYPE_XDP_TX derives the DMA address with
page_pool_get_dma_addr(), reading an uninitialized page->dma_addr, so
the device DMAs from a bogus address (corrupt TX, or an IOMMU fault).

Pick the TX buffer type from the frame's memory type: keep
PRUETH_TX_BUFF_TYPE_XDP_TX for page_pool frames and use
PRUETH_TX_BUFF_TYPE_XDP_NDO for the cloned zero-copy frame, which is then
DMA mapped through the NDO path and unmapped on completion.

While at it, fix the page_pool XDP_TX completion path. A
PRUETH_TX_BUFF_TYPE_XDP_TX frame carries a page_pool-owned DMA mapping
(established against rx_chn->dma_dev), yet prueth_xmit_free()
unconditionally calls dma_unmap_single() on it with tx_chn->dma_dev,
tearing down a mapping the driver does not own; xdp_return_frame()
already recycles the page back to the pool. Tag such frames with a
dedicated PRUETH_SWDATA_XDPF_TX type so the completion path skips the
unmap, the same way PRUETH_SWDATA_XSK buffers are handled.

Fixes: 7a64bb388df3 ("net: ti: icssg-prueth: Add AF_XDP zero copy for RX")
Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
v2:
 - fold in the page_pool XDP_TX completion-path unmap fix raised by
   Meghana Malladi: tag page_pool TX frames with PRUETH_SWDATA_XDPF_TX
   so prueth_xmit_free() skips dma_unmap_single() on a pool-owned
   mapping; xdp_return_frame() already recycles the page.
 - add Fixes: 62aa3246f462 for that path.
 - no change to the original zero-copy fix.
v1: https://lore.kernel.org/netdev/20260620213756.87499-1-devnexen@gmail.com
 drivers/net/ethernet/ti/icssg/icssg_common.c | 20 +++++++++++++++++---
 drivers/net/ethernet/ti/icssg/icssg_prueth.h |  1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 82ddef9c17d5..96c8bf5ef671 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -185,7 +185,7 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 	first_desc = desc;
 	next_desc = first_desc;
 	swdata = cppi5_hdesc_get_swdata(first_desc);
-	if (swdata->type == PRUETH_SWDATA_XSK)
+	if (swdata->type == PRUETH_SWDATA_XSK || swdata->type == PRUETH_SWDATA_XDPF_TX)
 		goto free_pool;
 
 	cppi5_hdesc_get_obuf(first_desc, &buf_dma, &buf_dma_len);
@@ -259,6 +259,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 			napi_consume_skb(skb, budget);
 			break;
 		case PRUETH_SWDATA_XDPF:
+		case PRUETH_SWDATA_XDPF_TX:
 			xdpf = swdata->data.xdpf;
 			dev_sw_netstats_tx_add(ndev, 1, xdpf->len);
 			total_bytes += xdpf->len;
@@ -769,7 +770,8 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
 	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
 	cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
 	swdata = cppi5_hdesc_get_swdata(first_desc);
-	swdata->type = PRUETH_SWDATA_XDPF;
+	swdata->type = buff_type == PRUETH_TX_BUFF_TYPE_XDP_TX ?
+		PRUETH_SWDATA_XDPF_TX : PRUETH_SWDATA_XDPF;
 	swdata->data.xdpf = xdpf;
 
 	/* Report BQL before sending the packet */
@@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(emac_xmit_xdp_frame);
  */
 static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len)
 {
+	enum prueth_tx_buff_type tx_buff_type;
 	struct net_device *ndev = emac->ndev;
 	struct netdev_queue *netif_txq;
 	int cpu = smp_processor_id();
@@ -826,11 +829,21 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len
 			goto drop;
 		}
 
+		/* In AF_XDP zero-copy mode xdp_convert_buff_to_frame()
+		 * clones the xsk buffer into a fresh MEM_TYPE_PAGE_ORDER0
+		 * page that is not DMA mapped. Such a frame must be mapped
+		 * via the NDO path; only a page pool-backed frame already
+		 * carries a usable page_pool DMA address.
+		 */
+		tx_buff_type = xdpf->mem_type == MEM_TYPE_PAGE_POOL ?
+				PRUETH_TX_BUFF_TYPE_XDP_TX :
+				PRUETH_TX_BUFF_TYPE_XDP_NDO;
+
 		q_idx = cpu % emac->tx_ch_num;
 		netif_txq = netdev_get_tx_queue(ndev, q_idx);
 		__netif_tx_lock(netif_txq, cpu);
 		result = emac_xmit_xdp_frame(emac, xdpf, q_idx,
-					     PRUETH_TX_BUFF_TYPE_XDP_TX);
+					     tx_buff_type);
 		__netif_tx_unlock(netif_txq);
 		if (result == ICSSG_XDP_CONSUMED) {
 			ndev->stats.tx_dropped++;
@@ -1395,6 +1408,7 @@ void prueth_tx_cleanup(void *data, dma_addr_t desc_dma)
 		dev_kfree_skb_any(skb);
 		break;
 	case PRUETH_SWDATA_XDPF:
+	case PRUETH_SWDATA_XDPF_TX:
 		xdpf = swdata->data.xdpf;
 		xdp_return_frame(xdpf);
 		break;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index df93d15c5b78..00bb760d68a9 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -153,6 +153,7 @@ enum prueth_swdata_type {
 	PRUETH_SWDATA_CMD,
 	PRUETH_SWDATA_XDPF,
 	PRUETH_SWDATA_XSK,
+	PRUETH_SWDATA_XDPF_TX,
 };
 
 enum prueth_tx_buff_type {
-- 
2.53.0

Re: [PATCH net v2] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path

[Meghana Malladi]

Few nitpicks,

On 6/23/26 16:52, David Carlier wrote:
> On XDP_TX from the zero-copy RX path, emac_run_xdp() converts the xsk
> buffer via xdp_convert_zc_to_xdp_frame(), which clones the data into a
> fresh MEM_TYPE_PAGE_ORDER0 page that is not DMA mapped. Transmitting it
> as PRUETH_TX_BUFF_TYPE_XDP_TX derives the DMA address with
> page_pool_get_dma_addr(), reading an uninitialized page->dma_addr, so
> the device DMAs from a bogus address (corrupt TX, or an IOMMU fault).
> 
> Pick the TX buffer type from the frame's memory type: keep
> PRUETH_TX_BUFF_TYPE_XDP_TX for page_pool frames and use
> PRUETH_TX_BUFF_TYPE_XDP_NDO for the cloned zero-copy frame, which is then
> DMA mapped through the NDO path and unmapped on completion.
> 
> While at it, fix the page_pool XDP_TX completion path. A
> PRUETH_TX_BUFF_TYPE_XDP_TX frame carries a page_pool-owned DMA mapping
> (established against rx_chn->dma_dev), yet prueth_xmit_free()
> unconditionally calls dma_unmap_single() on it with tx_chn->dma_dev,
> tearing down a mapping the driver does not own; xdp_return_frame()
> already recycles the page back to the pool. Tag such frames with a
> dedicated PRUETH_SWDATA_XDPF_TX type so the completion path skips the
> unmap, the same way PRUETH_SWDATA_XSK buffers are handled.
> 
> Fixes: 7a64bb388df3 ("net: ti: icssg-prueth: Add AF_XDP zero copy for RX")
> Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> v2:
>   - fold in the page_pool XDP_TX completion-path unmap fix raised by
>     Meghana Malladi: tag page_pool TX frames with PRUETH_SWDATA_XDPF_TX
>     so prueth_xmit_free() skips dma_unmap_single() on a pool-owned
>     mapping; xdp_return_frame() already recycles the page.
>   - add Fixes: 62aa3246f462 for that path.
>   - no change to the original zero-copy fix.
> v1: https://lore.kernel.org/netdev/20260620213756.87499-1-devnexen@gmail.com
>   drivers/net/ethernet/ti/icssg/icssg_common.c | 20 +++++++++++++++++---
>   drivers/net/ethernet/ti/icssg/icssg_prueth.h |  1 +
>   2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 82ddef9c17d5..96c8bf5ef671 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -185,7 +185,7 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
>   	first_desc = desc;
>   	next_desc = first_desc;
>   	swdata = cppi5_hdesc_get_swdata(first_desc);
> -	if (swdata->type == PRUETH_SWDATA_XSK)
> +	if (swdata->type == PRUETH_SWDATA_XSK || swdata->type == PRUETH_SWDATA_XDPF_TX)

line length crosses 80 characters

>   		goto free_pool;
>   
>   	cppi5_hdesc_get_obuf(first_desc, &buf_dma, &buf_dma_len);
> @@ -259,6 +259,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
>   			napi_consume_skb(skb, budget);
>   			break;
>   		case PRUETH_SWDATA_XDPF:
> +		case PRUETH_SWDATA_XDPF_TX:
>   			xdpf = swdata->data.xdpf;
>   			dev_sw_netstats_tx_add(ndev, 1, xdpf->len);
>   			total_bytes += xdpf->len;
> @@ -769,7 +770,8 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
>   	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>   	cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
>   	swdata = cppi5_hdesc_get_swdata(first_desc);
> -	swdata->type = PRUETH_SWDATA_XDPF;
> +	swdata->type = buff_type == PRUETH_TX_BUFF_TYPE_XDP_TX ?
> +		PRUETH_SWDATA_XDPF_TX : PRUETH_SWDATA_XDPF;

Use braces for the condition please

>   	swdata->data.xdpf = xdpf;
>   
>   	/* Report BQL before sending the packet */
> @@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(emac_xmit_xdp_frame);
>    */
>   static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len)
>   {
> +	enum prueth_tx_buff_type tx_buff_type;
>   	struct net_device *ndev = emac->ndev;
>   	struct netdev_queue *netif_txq;
>   	int cpu = smp_processor_id();
> @@ -826,11 +829,21 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len
>   			goto drop;
>   		}
>   
> +		/* In AF_XDP zero-copy mode xdp_convert_buff_to_frame()
> +		 * clones the xsk buffer into a fresh MEM_TYPE_PAGE_ORDER0
> +		 * page that is not DMA mapped. Such a frame must be mapped
> +		 * via the NDO path; only a page pool-backed frame already
> +		 * carries a usable page_pool DMA address.
> +		 */
> +		tx_buff_type = xdpf->mem_type == MEM_TYPE_PAGE_POOL ?
> +				PRUETH_TX_BUFF_TYPE_XDP_TX :
> +				PRUETH_TX_BUFF_TYPE_XDP_NDO;
> +
>   		q_idx = cpu % emac->tx_ch_num;
>   		netif_txq = netdev_get_tx_queue(ndev, q_idx);
>   		__netif_tx_lock(netif_txq, cpu);
>   		result = emac_xmit_xdp_frame(emac, xdpf, q_idx,
> -					     PRUETH_TX_BUFF_TYPE_XDP_TX);
> +					     tx_buff_type);
>   		__netif_tx_unlock(netif_txq);
>   		if (result == ICSSG_XDP_CONSUMED) {
>   			ndev->stats.tx_dropped++;
> @@ -1395,6 +1408,7 @@ void prueth_tx_cleanup(void *data, dma_addr_t desc_dma)
>   		dev_kfree_skb_any(skb);
>   		break;
>   	case PRUETH_SWDATA_XDPF:
> +	case PRUETH_SWDATA_XDPF_TX:
>   		xdpf = swdata->data.xdpf;
>   		xdp_return_frame(xdpf);
>   		break;
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index df93d15c5b78..00bb760d68a9 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -153,6 +153,7 @@ enum prueth_swdata_type {
>   	PRUETH_SWDATA_CMD,
>   	PRUETH_SWDATA_XDPF,
>   	PRUETH_SWDATA_XSK,
> +	PRUETH_SWDATA_XDPF_TX,
>   };
>   
>   enum prueth_tx_buff_type {

Reviewed-by: Meghana Malladi <m-malladi@ti.com>

Re: [PATCH net v2] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path

[sashiko-bot]

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

New issues:
- [Critical] Uninitialized descriptor usage leads to arbitrary DMA unmapping and memory corruption

Pre-existing issues:
- [Critical] Double free and memory leak on XDP_TX failure path for zero-copy frames
- [High] DMA API Violation: Freeing memory before unmapping DMA
- [High] Missing dma_sync_single_for_device() causes stale data transmission
--

commit 87bb8b22d4b9f56b9ec402460012e589c44163a5
Author: David Carlier <devnexen@gmail.com>

net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path

This commit addresses an issue with XDP_TX on zero-copy RX frames by using
the NDO path to DMA map cloned frames instead of treating them as page pool
buffers. It also introduces PRUETH_SWDATA_XDPF_TX to skip DMA unmapping for
page pool frames on the completion path.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 82ddef9c17d54..96c8bf5ef6719 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -185,7 +185,7 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
>  	first_desc = desc;
>  	next_desc = first_desc;
>  	swdata = cppi5_hdesc_get_swdata(first_desc);
> -	if (swdata->type == PRUETH_SWDATA_XSK)
> +	if (swdata->type == PRUETH_SWDATA_XSK || swdata->type == PRUETH_SWDATA_XDPF_TX)
>  		goto free_pool;
>  
>  	cppi5_hdesc_get_obuf(first_desc, &buf_dma, &buf_dma_len);

[Severity: Critical]
Does this error path properly handle uninitialized descriptors?

In emac_xmit_xdp_frame(), a new descriptor is allocated via
k3_cppi_desc_pool_alloc(), which uses gen_pool_alloc() and does not zero the
returned memory.

This patch allows emac_xmit_xdp_frame() to take the dma_map_single() branch
for non-page-pool XDP frames. If that mapping fails, the code jumps to
drop_free_descs and passes first_desc to prueth_xmit_free().

Because the descriptor is completely uninitialized at this point (even
cppi5_hdesc_init() has not been called), prueth_xmit_free() will read garbage
state from a previous packet.

Could this read a stale swdata->type and org_buf_ptr, unmapping an active
DMA buffer belonging to another packet, and following any garbage
next_desc_dma link to potentially double-free active descriptors?

[ ... ]

> @@ -259,6 +259,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
>  			napi_consume_skb(skb, budget);
>  			break;
>  		case PRUETH_SWDATA_XDPF:
> +		case PRUETH_SWDATA_XDPF_TX:
>  			xdpf = swdata->data.xdpf;
>  			dev_sw_netstats_tx_add(ndev, 1, xdpf->len);
>  			total_bytes += xdpf->len;

[Severity: High]
This is a pre-existing issue, but does this violate the DMA API by freeing
memory before unmapping it?

In emac_tx_complete_packets() and prueth_tx_cleanup(), when cleaning up
PRUETH_SWDATA_XDPF frames, the code calls xdp_return_frame() to free the
underlying memory back to the allocator.

Later in the function, it calls prueth_xmit_free() which subsequently
performs dma_unmap_single() on the buffer.

Could this race condition allow the memory to be reallocated to another
context, causing the delayed DMA unmap operation to corrupt IOMMU state
or free SWIOTLB bounce buffers out from under the new owner?

[ ... ]

> @@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(emac_xmit_xdp_frame);
>   */
>  static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len)
>  {
> +	enum prueth_tx_buff_type tx_buff_type;
>  	struct net_device *ndev = emac->ndev;
>  	struct netdev_queue *netif_txq;
>  	int cpu = smp_processor_id();
> @@ -826,11 +829,21 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len
>  			goto drop;
>  		}
>  
> +		/* In AF_XDP zero-copy mode xdp_convert_buff_to_frame()
> +		 * clones the xsk buffer into a fresh MEM_TYPE_PAGE_ORDER0
> +		 * page that is not DMA mapped. Such a frame must be mapped
> +		 * via the NDO path; only a page pool-backed frame already
> +		 * carries a usable page_pool DMA address.
> +		 */
> +		tx_buff_type = xdpf->mem_type == MEM_TYPE_PAGE_POOL ?
> +				PRUETH_TX_BUFF_TYPE_XDP_TX :
> +				PRUETH_TX_BUFF_TYPE_XDP_NDO;

[Severity: High]
This is a pre-existing issue, but is a dma_sync_single_for_device() call
missing here?

For page pool-backed frames (PRUETH_TX_BUFF_TYPE_XDP_TX),
emac_xmit_xdp_frame() avoids calling dma_map_single() and derives the DMA
address using page_pool_get_dma_addr(). It then submits this address directly
to the TX ring.

If the XDP program modified the packet (e.g., swapping MAC addresses or
altering IP headers), those modifications reside in the CPU cache.

Because the driver fails to call dma_sync_single_for_device() before
transmission, could the NIC's DMA engine transmit stale, unmodified packet
contents on non-cache-coherent architectures?

> +
>  		q_idx = cpu % emac->tx_ch_num;
>  		netif_txq = netdev_get_tx_queue(ndev, q_idx);
>  		__netif_tx_lock(netif_txq, cpu);
>  		result = emac_xmit_xdp_frame(emac, xdpf, q_idx,
> -					     PRUETH_TX_BUFF_TYPE_XDP_TX);
> +					     tx_buff_type);
>  		__netif_tx_unlock(netif_txq);
>  		if (result == ICSSG_XDP_CONSUMED) {
>  			ndev->stats.tx_dropped++;

[Severity: Critical]
This is a pre-existing issue, but does this error path cause a double free
and a memory leak?

When emac_run_xdp() processes an XDP_TX for an AF_XDP zero-copy buffer,
xdp_convert_buff_to_frame() frees the original zero-copy buffer via
xsk_buff_free() and allocates a new MEM_TYPE_PAGE_ORDER0 page.

If the transmission via emac_xmit_xdp_frame() fails (e.g., due to a full TX
ring), it returns ICSSG_XDP_CONSUMED. The newly allocated xdpf is not freed
here, causing a memory leak.

Furthermore, emac_run_xdp() propagates ICSSG_XDP_CONSUMED back to
emac_rx_packet_zc(). emac_rx_packet_zc() handles ICSSG_XDP_CONSUMED by
calling xsk_buff_free(xdp) on the original buffer again.

Since the buffer was already freed during the cloning step, wouldn't this
trigger a double free?

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