[PATCH] net: mvpp2: fix skb not reflecting XDP modifications on XDP_PASS

[PATCH] net: mvpp2: fix skb not reflecting XDP modifications on XDP_PASS

[Til Kaiser]

When an XDP program uses bpf_xdp_adjust_head() or bpf_xdp_adjust_tail()
and then returns XDP_PASS, the driver ignores the updated xdp_buff
pointers and builds the skb from hardcoded offsets derived from the
original RX descriptor. This means any packet modifications made by the
XDP program are silently discarded before the skb reaches the network
stack.

Fix this by saving the post-XDP data pointer offset and length back into
rx_offset and rx_bytes when XDP_PASS is returned, so the subsequent
skb_reserve()/skb_put() calls use the actual packet geometry as seen by
the XDP program. When no XDP program is loaded the variables retain
their original descriptor-derived values and the non-XDP fast path is
unaffected.

Fixes: 07dd0a7aae7f ("mvpp2: add basic XDP support")
Signed-off-by: Til Kaiser <mail@tk154.de>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index f442b874bb59..27d4feebcb59 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3920,7 +3920,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 		unsigned int frag_size;
 		dma_addr_t dma_addr;
 		phys_addr_t phys_addr;
-		int pool, rx_bytes, err, ret;
+		int pool, rx_bytes, rx_offset, err, ret;
 		struct page *page;
 		void *data;
 
@@ -3933,6 +3933,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 		rx_status = mvpp2_rxdesc_status_get(port, rx_desc);
 		rx_bytes = mvpp2_rxdesc_size_get(port, rx_desc);
 		rx_bytes -= MVPP2_MH_SIZE;
+		rx_offset = MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
 		dma_addr = mvpp2_rxdesc_dma_addr_get(port, rx_desc);
 
 		pool = (rx_status & MVPP2_RXD_BM_POOL_ID_MASK) >>
@@ -3998,6 +3999,10 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 				continue;
 			}
 
+			/* Update offset and length to reflect any XDP adjustments. */
+			rx_offset = xdp.data     - data;
+			rx_bytes  = xdp.data_end - xdp.data;
+
 			metasize = xdp.data - xdp.data_meta;
 		}
 
@@ -4036,7 +4041,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 		ps.rx_packets++;
 		ps.rx_bytes += rx_bytes;
 
-		skb_reserve(skb, MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM);
+		skb_reserve(skb, rx_offset);
 		skb_put(skb, rx_bytes);
 		if (metasize)
 			skb_metadata_set(skb, metasize);
-- 
2.54.0

Re: [PATCH] net: mvpp2: fix skb not reflecting XDP modifications on XDP_PASS

[sashiko-bot]

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

New issues:
- [High] The patch passes an unchecked XDP-modified length to `skb_put()`, introducing a deterministic kernel panic (`skb_over_panic`) if the XDP program grows the packet.
- [Medium] RX byte statistics inconsistently track XDP modifications.

Pre-existing issues:
- [Critical] Freed data buffers are returned to the hardware Buffer Manager (BM) pool during the RX buffer refill error path, causing severe memory corruption.
- [High] The DMA sync operation misses the packet tail due to an unhandled headroom offset, leading to potential data corruption.
- [High] XDP buffer initialization unconditionally uses `PAGE_SIZE`, allowing out-of-bounds writes on short buffers.
- [High] Hardware checksum status is blindly trusted even after XDP modifications, bypassing network stack validation.
--

commit 07608dfe3a3f044abc14018615b3941358f7641f
Author: Til Kaiser <mail@tk154.de>

net: mvpp2: fix skb not reflecting XDP modifications on XDP_PASS

When an XDP program uses bpf_xdp_adjust_head() or bpf_xdp_adjust_tail()
and then returns XDP_PASS, the driver ignores the updated xdp_buff
pointers and builds the skb from hardcoded offsets derived from the
original RX descriptor. This means any packet modifications made by the
XDP program are silently discarded before the skb reaches the network
stack.

Fix this by saving the post-XDP data pointer offset and length back into
rx_offset and rx_bytes when XDP_PASS is returned, so the subsequent
skb_reserve()/skb_put() calls use the actual packet geometry as seen by
the XDP program. When no XDP program is loaded the variables retain
their original descriptor-derived values and the non-XDP fast path is
unaffected.

> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index f442b874bb593..27d4feebcb595 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -3933,6 +3933,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
>  		rx_status = mvpp2_rxdesc_status_get(port, rx_desc);
>  		rx_bytes = mvpp2_rxdesc_size_get(port, rx_desc);
>  		rx_bytes -= MVPP2_MH_SIZE;
> +		rx_offset = MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
>  		dma_addr = mvpp2_rxdesc_dma_addr_get(port, rx_desc);
>  
>  		pool = (rx_status & MVPP2_RXD_BM_POOL_ID_MASK) >>
>  			MVPP2_RXD_BM_POOL_ID_OFFS;
>  		bm_pool = &port->priv->bm_pools[pool];
>  
>  		if (port->priv->percpu_pools) {
>  			pp = port->priv->page_pool[pool];
>  			dma_dir = page_pool_get_dma_dir(pp);
>  		} else {
>  			dma_dir = DMA_FROM_DEVICE;
>  		}
>  
>  		dma_sync_single_for_cpu(dev->dev.parent, dma_addr,
>  					rx_bytes + MVPP2_MH_SIZE,
>  					dma_dir);

[Severity: High]
This is a pre-existing issue, but does this DMA sync miss the end of the
packet?

Since the payload is written by the hardware at an offset of MVPP2_MH_SIZE +
MVPP2_SKB_HEADROOM from dma_addr, syncing only rx_bytes + MVPP2_MH_SIZE seems
to sync the empty headroom at the beginning but skip the last
MVPP2_SKB_HEADROOM bytes of the packet.

On non-coherent architectures, could this cause the CPU to read stale cached
data for the tail of the packet, leading to data corruption?

[ ... ]

>  		if (xdp_prog) {
>  			struct xdp_rxq_info *xdp_rxq;
>  
>  			if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
>  				xdp_rxq = &rxq->xdp_rxq_short;
>  			else
>  				xdp_rxq = &rxq->xdp_rxq_long;
>  
>  			xdp_init_buff(&xdp, PAGE_SIZE, xdp_rxq);

[Severity: High]
This isn't a bug introduced by this patch, but does unconditionally using
PAGE_SIZE here allow out-of-bounds writes?

The actual allocated fragment size (bm_pool->frag_size) can be smaller than a
page, such as for the short packet pool. If an XDP program uses
bpf_xdp_adjust_tail() to grow the packet up to PAGE_SIZE, could this write past
the end of the allocated buffer and corrupt adjacent memory?

>  			xdp_prepare_buff(&xdp, data,
>  					 MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM,
>  					 rx_bytes, true);
>  
>  			ret = mvpp2_run_xdp(port, xdp_prog, &xdp, pp, &ps);
>  
>  			if (ret) {
>  				xdp_ret |= ret;
>  				err = mvpp2_rx_refill(port, bm_pool, pp, pool);
>  				if (err) {
>  					netdev_err(port->dev, "failed to refill BM pools\n");
>  					goto err_drop_frame;
>  				}
>  
>  				ps.rx_packets++;
>  				ps.rx_bytes += rx_bytes;
>  				continue;
>  			}
>  
> +			/* Update offset and length to reflect any XDP adjustments. */
> +			rx_offset = xdp.data     - data;
> +			rx_bytes  = xdp.data_end - xdp.data;

[Severity: Medium]
Will this result in inconsistent tracking of RX byte statistics?

If the XDP program returns XDP_TX or XDP_DROP, the original pre-XDP length is
added to ps.rx_bytes earlier in this loop. When it returns XDP_PASS, the
updated post-XDP length is now stored in rx_bytes and added to ps.rx_bytes at
the end of the loop.

Could this mix wire lengths and post-XDP lengths in the statistics depending
on the verdict?

>  
>  			metasize = xdp.data - xdp.data_meta;
>  		}
>  
>  		if (frag_size)
>  			skb = build_skb(data, frag_size);
>  		else
>  			skb = slab_build_skb(data);
>  		if (!skb) {
>  			netdev_warn(port->dev, "skb build failed\n");
>  			goto err_drop_frame;
>  		}
>  
>  		/* If we have RX hardware timestamping enabled, grab the
>  		 * timestamp from the queue and convert.
>  		 */
>  		if (mvpp22_rx_hwtstamping(port)) {
>  			timestamp = le32_to_cpu(rx_desc->pp22.timestamp);
>  			mvpp22_tai_tstamp(port->priv->tai, timestamp,
>  					 skb_hwtstamps(skb));
>  		}
>  
>  		err = mvpp2_rx_refill(port, bm_pool, pp, pool);
>  		if (err) {
>  			netdev_err(port->dev, "failed to refill BM pools\n");
>  			dev_kfree_skb_any(skb);
>  			goto err_drop_frame;
>  		}

[Severity: Critical]
This is a pre-existing issue, but does this error path lead to memory
corruption?

If mvpp2_rx_refill() fails, dev_kfree_skb_any(skb) is called which frees the
SKB and its underlying data buffer back to the kernel allocator. However, the
path then jumps to err_drop_frame, which calls mvpp2_bm_pool_put() with the
same physical address.

Since this returns the freed memory back to the hardware RX pool, could the
hardware DMA new packets into memory that the kernel considers free and may
have reallocated to other subsystems?

[ ... ]

>  		ps.rx_packets++;
>  		ps.rx_bytes += rx_bytes;
>  
> -		skb_reserve(skb, MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM);
> +		skb_reserve(skb, rx_offset);
>  		skb_put(skb, rx_bytes);

[Severity: High]
Does this updated skb_put() call risk triggering a deterministic kernel panic?

Because xdp_init_buff() incorrectly advertises a capacity of PAGE_SIZE even
for short buffers, an XDP program can legitimately grow the packet length
beyond frag_size. When this unchecked rx_bytes length is passed to skb_put(),
if it exceeds the SKB's tailroom (initialized with frag_size in build_skb()),
wouldn't this trigger skb_over_panic()?

>  		if (metasize)
>  			skb_metadata_set(skb, metasize);
>  		skb->ip_summed = mvpp2_rx_csum(port, rx_status);

[Severity: High]
This isn't a bug introduced by this patch, but should the hardware checksum
status be trusted if the packet was modified by XDP?

If an XDP program modifies the packet payload or changes packet headers (such
as through bpf_xdp_adjust_head()), the hardware checksum is no longer valid.
By unconditionally setting skb->ip_summed based on the original hardware
state, could this falsely tell the network stack that the checksum is valid
and allow corrupted or incorrectly modified packets to bypass software
checksum validation?

>  		skb->protocol = eth_type_trans(skb, dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524122334.1730451-1-mail@tk154.de?part=1

Re: [PATCH] net: mvpp2: fix skb not reflecting XDP modifications on XDP_PASS

[Jakub Kicinski]

On Sun, 24 May 2026 14:23:34 +0200 Til Kaiser wrote:
> When an XDP program uses bpf_xdp_adjust_head() or bpf_xdp_adjust_tail()
> and then returns XDP_PASS, the driver ignores the updated xdp_buff
> pointers and builds the skb from hardcoded offsets derived from the
> original RX descriptor. This means any packet modifications made by the
> XDP program are silently discarded before the skb reaches the network
> stack.
> 
> Fix this by saving the post-XDP data pointer offset and length back into
> rx_offset and rx_bytes when XDP_PASS is returned, so the subsequent
> skb_reserve()/skb_put() calls use the actual packet geometry as seen by
> the XDP program. When no XDP program is loaded the variables retain
> their original descriptor-derived values and the non-XDP fast path is
> unaffected.
> 
> Fixes: 07dd0a7aae7f ("mvpp2: add basic XDP support")
> Signed-off-by: Til Kaiser <mail@tk154.de>

Other than the checksum complaint Sashiko review looks legit.
Please respin and try to fix more of this issues in one series.

Could you try to run:
tools/testing/selftests/drivers/net/xdp.py
it may surface other common driver bugs.
See: tools/testing/selftests/drivers/net/README.rst for instructions
-- 
pw-bot: cr