* [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path
@ 2026-06-25 6:31 David Carlier
2026-06-26 6:31 ` sashiko-bot
2026-06-30 1:12 ` Jakub Kicinski
0 siblings, 2 replies; 3+ messages in thread
From: David Carlier @ 2026-06-25 6:31 UTC (permalink / raw)
To: danishanwar, rogerq, andrew+netdev, netdev
Cc: davem, edumazet, kuba, pabeni, horms, m-malladi, hawk,
john.fastabend, sdf, ast, daniel, bpf, linux-arm-kernel,
linux-kernel, stable, 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>
Reviewed-by: Meghana Malladi <m-malladi@ti.com>
---
v3:
- address Meghana Malladi review nits: split the prueth_xmit_free()
guard to stay under 80 columns, parenthesize the swdata->type
ternary (and the matching tx_buff_type one for consistency).
- no functional change; carry Reviewed-by.
v2: https://lore.kernel.org/netdev/20260623112225.303930-1-devnexen@gmail.com
v1: https://lore.kernel.org/netdev/20260620213756.87499-1-devnexen@gmail.com
drivers/net/ethernet/ti/icssg/icssg_common.c | 21 +++++++++++++++++---
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 1 +
2 files changed, 19 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..64ae3704481e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -185,7 +185,8 @@ 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 +260,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 +771,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 +807,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 +830,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 +1409,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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path
2026-06-25 6:31 [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path David Carlier
@ 2026-06-26 6:31 ` sashiko-bot
2026-06-30 1:12 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-26 6:31 UTC (permalink / raw)
To: David Carlier; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Uninitialized descriptor memory used for DMA unmapping in error paths.
- [High] DMA API violation: memory is freed before the DMA mapping is torn down.
- [High] Memory leak and double free when XDP_TX or XDP_REDIRECT fails in AF_XDP zero-copy mode.
- [High] Missing DMA synchronization for transmitted Page Pool XDP_TX frames.
- [High] Use-After-Free reading xdpf->len after handing the frame to the hardware TX queue.
- [Medium] Concurrent non-atomic increments to shared global network statistics.
--
commit dc2f8356525786b4ee8c94beae174920f574ebd4
Author: David Carlier <devnexen@gmail.com>
net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path
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).
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 4a7d1a6f470b1..1d9ae764ce88f 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
[ ... ]
> @@ -259,6 +260,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 the
memory before tearing down the DMA mapping?
In both emac_tx_complete_packets() and prueth_tx_cleanup(), the driver frees
the XDP frame via xdp_return_frame() before calling prueth_xmit_free(), which
performs the actual dma_unmap_single().
The physical memory is returned to the allocator and can be immediately reused
by another subsystem while the device or IOMMU still has an active mapping for
it, creating a use-after-free window.
[ ... ]
> @@ -768,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;
[Severity: Critical]
This is a pre-existing issue, but does the error path pass an uninitialized
first_desc to the cleanup function?
If dma_map_single() or virt_to_head_page() fails in emac_xmit_xdp_frame(), the
driver jumps to the drop_free_descs error path and passes the newly allocated
first_desc to prueth_xmit_free().
Because the descriptor is allocated via k3_cppi_desc_pool_alloc() without
zeroing the memory, and it is passed before swdata->type and obuf are
initialized, prueth_xmit_free() will read stale swdata->type and garbage
buf_dma/buf_dma_len from a previous packet.
It then calls dma_unmap_single() with this garbage DMA address, potentially
unmapping memory actively used by other subsystems.
[Severity: High]
This isn't a bug introduced by this patch, but does the page pool frame
transmission miss a DMA synchronization?
When buff_type == PRUETH_TX_BUFF_TYPE_XDP_TX, emac_xmit_xdp_frame()
retrieves the DMA address via page_pool_get_dma_addr() and passes it to the
TX hardware without calling dma_sync_single_for_device().
Because the hardware is not cache-coherent, the device may transmit stale data
from RAM instead of the modified cache lines from the BPF program.
[ ... ]
> @@ -825,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++;
[Severity: High]
This is a pre-existing issue, but does this leak memory and cause a
double-free when XDP_TX fails in AF_XDP zero-copy mode?
When an AF_XDP zero-copy buffer is handled, xdp_convert_buff_to_frame()
allocates a new page for the XDP frame and frees the original xsk_buff via
xsk_buff_free().
If the transmission fails and emac_xmit_xdp_frame() returns
ICSSG_XDP_CONSUMED, emac_run_xdp() propagates ICSSG_XDP_CONSUMED to the
caller but never frees the newly allocated xdpf page.
Furthermore, the caller emac_rx_packet_zc() unconditionally calls
xsk_buff_free(xdp) when receiving ICSSG_XDP_CONSUMED, double-freeing the
original xdp buffer.
[Severity: Medium]
This isn't a bug introduced by this patch, but does this perform an unprotected
increment to a shared global statistic?
The driver performs unprotected increments like ndev->stats.tx_dropped++
in multiple contexts, including emac_run_xdp() and icssg_ndo_start_xmit().
Because multiple CPUs can execute these paths concurrently, these
read-modify-write operations on the shared net_device struct can result in
torn reads and lost updates.
[Severity: High]
This is a pre-existing issue, but is there a use-after-free when reading
xdpf->len after handing the frame to the hardware TX queue?
In emac_run_xdp(), the driver passes xdpf to emac_xmit_xdp_frame(), which
enqueues it to the hardware DMA ring. Once pushed, the hardware could transmit
it immediately, allowing the TX completion interrupt on another CPU to pop the
descriptor and free xdpf.
Following this handoff, emac_run_xdp() accesses xdpf->len for statistics
accounting (dev_sw_netstats_rx_add), resulting in a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625063121.24746-1-devnexen@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path
2026-06-25 6:31 [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path David Carlier
2026-06-26 6:31 ` sashiko-bot
@ 2026-06-30 1:12 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-06-30 1:12 UTC (permalink / raw)
To: David Carlier
Cc: danishanwar, rogerq, andrew+netdev, netdev, davem, edumazet,
pabeni, horms, m-malladi, hawk, john.fastabend, sdf, ast, daniel,
bpf, linux-arm-kernel, linux-kernel, stable
On Thu, 25 Jun 2026 07:31:21 +0100 David Carlier wrote:
> + PRUETH_SWDATA_XDPF_TX,
Sorry for the naming nit on v3 but XDPF_TX is ambiguous, I spent
a bunch of time digging thru the driver. It should really be called
XDPF_LOCAL or some such to indicate that the frame originates locally.
XDPF_TX is ambiguous.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-30 1:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 6:31 [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path David Carlier
2026-06-26 6:31 ` sashiko-bot
2026-06-30 1:12 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.