[PATCH net 1/2] net: pass the XDP action to generic_xdp_tx() for tracing

[PATCH net 1/2] net: pass the XDP action to generic_xdp_tx() for tracing

[Vladimir Oltean]

When generic_xdp_tx() fails to transmit an skb, it reports the drop
through trace_xdp_exception() with the action hardcoded to XDP_TX.
That was right when generic XDP was added, and XDP_TX was the only
caller. Since then, two XDP_REDIRECT paths have started to transmit
through generic_xdp_tx() as well:

- xdp_do_generic_redirect(), for a redirect to an ifindex
- dev_map_generic_redirect(), for DEVMAP and DEVMAP_HASH redirects,
  including broadcast

Drops on those paths are reported as XDP_TX, an action which the
program never returned. Tools which count xdp:xdp_exception events per
action, such as the samples/bpf programs built on xdp_sample, therefore
attribute generic XDP redirect drops to the wrong action. Drivers with
native XDP already report XDP_TX and XDP_REDIRECT drops separately.

Add an "act" argument to generic_xdp_tx(), and have each caller pass
the action that led to the transmission: XDP_TX from do_xdp_generic(),
XDP_REDIRECT from the two redirect paths. generic_xdp_tx() is not
exported, so these are all of its users.

Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 6d5fc1957989 ("xdp: Fix handling of devmap in generic XDP")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/netdevice.h | 3 ++-
 kernel/bpf/devmap.c       | 2 +-
 net/core/dev.c            | 7 ++++---
 net/core/filter.c         | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c0ae442c4344..b988afe797c3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4355,7 +4355,8 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
 
 u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
 			     const struct bpf_prog *xdp_prog);
-void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog);
+void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog,
+		    u32 act);
 int do_xdp_generic(const struct bpf_prog *xdp_prog, struct sk_buff **pskb);
 int netif_rx(struct sk_buff *skb);
 int __netif_rx(struct sk_buff *skb);
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 1b8f0d7f8455..c73e8797c0fc 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -731,7 +731,7 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
 		return 0;
 
 	skb->dev = dst->dev;
-	generic_xdp_tx(skb, xdp_prog);
+	generic_xdp_tx(skb, xdp_prog, XDP_REDIRECT);
 
 	return 0;
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 3ac700d37e59..88860185f583 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5851,7 +5851,8 @@ static u32 netif_receive_generic_xdp(struct sk_buff **pskb,
  * and DDOS attacks will be more effective. In-driver-XDP use dedicated TX
  * queues, so they do not have this starvation issue.
  */
-void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
+void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog,
+		    u32 act)
 {
 	struct net_device *dev = skb->dev;
 	struct netdev_queue *txq;
@@ -5868,7 +5869,7 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
 	}
 	HARD_TX_UNLOCK(dev, txq);
 	if (free_skb) {
-		trace_xdp_exception(dev, xdp_prog, XDP_TX);
+		trace_xdp_exception(dev, xdp_prog, act);
 		dev_core_stats_tx_dropped_inc(dev);
 		kfree_skb(skb);
 	}
@@ -5896,7 +5897,7 @@ int do_xdp_generic(const struct bpf_prog *xdp_prog, struct sk_buff **pskb)
 					goto out_redir;
 				break;
 			case XDP_TX:
-				generic_xdp_tx(*pskb, xdp_prog);
+				generic_xdp_tx(*pskb, xdp_prog, XDP_TX);
 				break;
 			}
 			bpf_net_ctx_clear(bpf_net_ctx);
diff --git a/net/core/filter.c b/net/core/filter.c
index 09c60a58c82a..7c6f8bcc6c29 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4821,7 +4821,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 
 		skb->dev = fwd;
 		_trace_xdp_redirect(dev, xdp_prog, ri->tgt_index);
-		generic_xdp_tx(skb, xdp_prog);
+		generic_xdp_tx(skb, xdp_prog, XDP_REDIRECT);
 		return 0;
 	}
 
-- 
2.43.0

[PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()

[Vladimir Oltean]

While auditing the limits imposed by the network stack on the number of
GSO segments, I found that a GSO super-skb can reach a driver's
ndo_start_xmit() with skb_shinfo(skb)->gso_segs set to 0, contrary to
what drivers might expect. This involves SKB_GSO_DODGY, and can be
easily reproduced on hardware.

"Dodgy" GSO skbs are skbs whose GSO metadata comes from a source that
the kernel does not trust. The main producer is virtio_net_hdr_to_skb(),
which translates the offload request in a struct virtio_net_hdr into
skb_shinfo() fields. It is used by tun, tap, AF_PACKET with
PACKET_VNET_HDR and virtio_net on RX. It only does basic sanity checks
(csum_start/csum_offset, gso_type, a minimum gso_size), then sets
gso_size and gso_type, adds SKB_GSO_DODGY and leaves gso_segs at 0
("Header must be checked, and gso_segs computed"). The rest of the
validation is deferred to the transmit path, and the "dodgy" flag
stays set afterwards unless the skb is actually segmented.

On the dev_queue_xmit() path, qdisc_pkt_len_segs_init() already
recomputes gso_segs for SKB_GSO_DODGY skbs. After that,
validate_xmit_skb() decides whether the xmit netdev is capable of
fulfilling an offload or whether it should be applied in the core.
For GSO, skb_shinfo(skb)->gso_type is verified bitwise against the
netdev features in net_gso_ok(): SKB_GSO_TCPV4 against NETIF_F_TSO,
SKB_GSO_UDP_L4 against NETIF_F_GSO_UDP_L4, etc. For the SKB_GSO_DODGY
gso_type bit, the corresponding feature is NETIF_F_GSO_ROBUST, which
means "give me the unvalidated super-skb anyway".

When the xmit netdev lacks a feature corresponding to a set bit in
gso_type, __skb_gso_segment() is called, which dispatches to the
protocol's net_offload :: gso_segment() method (inet_gso_segment() ->
tcp4_gso_segment() etc). Physical NICs do not advertise
NETIF_F_GSO_ROBUST, so for them the SKB_GSO_DODGY bit alone is enough
to get here, even if they support TSO. tcp_gso_segment() and
__udp_gso_segment() then have this code:

	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
		/* Packet is from an untrusted source, reset gso_segs. */

		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);

		segs = NULL;
		goto out;
	}

This fixes up the gso_segs value and returns NULL, which means that
the protocol did not segment the super-skb, but that this is not an
error either, so execution continues. The "dodgy" super-skb reaches the
driver's ndo_start_xmit() with a valid gso_segs value and gets
segmented there or in hardware.

Generic XDP skips all of this. Instead of calling dev_queue_xmit() or
dev_direct_xmit(), the XDP_TX and XDP_REDIRECT actions end up in
generic_xdp_tx(), which calls netdev_start_xmit() directly. There are
three callers:
- do_xdp_generic(), for XDP_TX
- xdp_do_generic_redirect(), for XDP_REDIRECT to an ifindex
- dev_map_generic_redirect(), for XDP_REDIRECT through a DEVMAP or
  DEVMAP_HASH map, including broadcast
None of them validates the skb against the features of the egress
device.

A process can reach this path by write()-ing a struct virtio_net_hdr
followed by an Ethernet frame to a TAP interface created by tun.c with
IFF_VNET_HDR. No virtual machine, virtio-net driver or vhost-net is
needed. Creating the interface and attaching the XDP program require
CAP_NET_ADMIN, but a persistent TAP interface whose owner was set with
TUNSETOWNER can then be written to by an unprivileged process. When the
virtio-net header requests GSO, tun_get_user() does not run tun's own
XDP program on an xdp_buff. It builds an skb and passes it to
do_xdp_generic() instead. vhost-net reaches the same code through
tun_xdp_one(). A program attached in generic mode runs on the same skb
from __netif_receive_skb_core(). In all cases, the program sees an skb
which virtio_net_hdr_to_skb() has just marked SKB_GSO_DODGY with
gso_segs = 0. If the program redirects it to a physical interface, that
interface's ndo_start_xmit() receives the skb unchanged.

The same happens inside a guest whose virtio_net interface runs a
generic XDP program, with GSO metadata coming from the host. virtio_net
only disables guest offloads (or refuses the program, if
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS is not negotiated) when XDP is
attached in native mode; a program attached in skb mode keeps receiving
GSO packets from the host.

macvtap and ipvtap (drivers/net/tap.c) also accept virtio-net headers
from user space, but are not affected. For them, a write from user
space is a transmit: tap_get_user() passes the skb to dev_queue_xmit()
on the macvtap/ipvtap interface, where qdisc_pkt_len_segs_init()
recomputes gso_segs. Only then can the skb reach an XDP hook, for
example when macvlan in bridge mode forwards it to a sibling interface.
Their RX path also runs generic XDP, but only on skbs that the lower
device received and handed up through macvlan_handle_frame() or
ipvlan_handle_frame(), before tap_handle_frame() queues them to user
space. Similarly, an skb that has gone through __dev_queue_xmit() once
(bridge, veth) normally has a valid gso_segs when it reaches generic XDP
on another interface.

The impact depends on how the egress driver uses gso_segs:

- Drivers which do software TSO and check for TX ring space with
  tso_count_descs() (enetc, fec, mvneta, mv643xx_eth, mvpp2), or with a
  similar gso_segs based estimate (thunder nicvf, octeontx2), compute
  that only nr_frags descriptors (or none) are needed. They then write
  descriptors for every real segment. xdp_ok_fwd_dev() limits skb->len
  to about the MTU, but gso_size can be as small as TCP_MIN_GSO_SIZE
  (8 bytes). With a 9000 byte MTU, enetc without hardware LSO needs
  2240 BDs for one such skb, so it overruns its 2048 entry ring and
  overwrites BDs that are still in use.
- nfp with the NFDK data path programs gso_segs into the TX descriptor
  as the total segment count, and sfc ef100 treats gso_segs == 0 as a
  non-TSO packet.
- igb, igc, ixgbe, i40e, iavf and libwx compute
  "bytecount += (gso_segs - 1) * hdr_len" and count gso_segs packets on
  completion. With gso_segs == 0, the TSO packet is counted as zero
  packets, and since (gso_segs - 1) is -1 after integer promotion, its
  byte count is skb->len - hdr_len. A completion interval with only
  such packets looks idle to the adaptive interrupt moderation of igb
  and igc ("packets == 0"). hns3 uses the same byte formula, but counts
  one packet per skb.
- Drivers that use gso_segs only for statistics and fall back to
  "gso_segs ?: 1" (e1000, e1000e, igbvf, r8152, lan78xx, ax88179_178a,
  be2net) count such an skb as one packet of skb->len bytes. This is
  the same convention as the core uses (qdisc_pkt_len_segs_init(),
  ip_select_ident_segs() callers): a gso_segs of 0 means "not filled
  in", not "no segments", and since any transmitted skb puts at least
  one frame on the wire, 1 is the smallest count that can be true.
  These drivers undercount, but the result is still a lower bound of
  what was sent, consistent between bytes and packets, and their
  descriptor use does not depend on gso_segs. Such a fallback would
  not help the software TSO drivers above, where an underestimate
  causes the overrun.
- Drivers that pass only gso_size and gso_type to a backend and
  advertise NETIF_F_GSO_ROBUST (such as virtio_net) never read
  gso_segs and are not affected.

Two places for the fix were considered: the XDP_REDIRECT callers of
generic_xdp_tx(), or generic_xdp_tx() itself.

XDP_TX is not affected by the gso_segs problem, because it transmits on
the interface which received the packet. The only interfaces whose RX
path hands generic XDP a GSO skb with gso_segs == 0 are those which
build it from a virtio-net header supplied by user space or a
hypervisor, such as tun and virtio_net. Their ndo_start_xmit() converts
the skb back into such a header using only gso_type and gso_size, and
never reads gso_segs. Physical NICs do not produce such skbs:
generic_xdp_install() turns off LRO and GRO_HW, and GSO skbs built by
GRO or by software interfaces like veth, lo or macvlan have a valid
gso_segs. Validating only in the redirect callers would therefore keep
XDP_TX on its current fast path.

However, validating in generic_xdp_tx() is still the better choice:

- Implementation-wise it is easier, since it covers all redirect paths
  at once. The alternative would be to add separate validation paths in
  xdp_do_generic_redirect() and in dev_map_generic_redirect(), which
  both call generic_xdp_tx(). There are also challenges to validation
  placement, which has to be done after skb->dev has been set to the
  egress device (which is already set properly in generic_xdp_tx()).
  Furthermore, generic_xdp_tx() is the only place which already
  transmits the skb, and so can handle what validation returns.
  validate_xmit_skb_list() may segment the skb, or free it and return
  NULL. If a caller validated the skb and it got segmented, the caller
  would have to transmit the segments itself. If it got freed, the
  caller could not report an error either, because do_xdp_generic()
  frees the skb again on error. Inside generic_xdp_tx(), the segments
  are transmitted one by one, and any left over when the queue stops are
  freed with kfree_skb_list().

- Validation is not only about gso_segs, and XDP_TX benefits from the
  rest of it too. With IFF_VNET_HDR, a tun interface accepts GSO
  packets from user space even if user space never enabled TSO with
  TUNSETOFFLOAD. XDP_TX on such a packet currently sends it back to
  user space with a GSO virtio-net header, an offload that user space
  never said it could handle. Checksum offload has the same problem.
  Validation fixes these cases by segmenting or checksumming the packet
  in software when the device lacks the feature.

So do in generic_xdp_tx() what __dev_queue_xmit() does before handing
the skb to the driver:

- Call qdisc_pkt_len_segs_init() first. It recomputes gso_segs for
  SKB_GSO_DODGY skbs and drops malformed ones. Without it,
  validate_xmit_skb() would compare the untrusted gso_segs = 0 against
  dev->gso_max_segs in gso_within_dev_limits(), and the check would
  always pass. The protocol's gso_segment() would then fix up gso_segs
  without comparing it against the limit, and with a 9000 byte MTU, a
  super-skb with about 1120 segments would reach a driver which set a
  lower limit with netif_set_tso_max_segs().

- Pick the TX queue before validation, as __dev_queue_xmit() does.
  netdev_core_pick_tx() doesn't operate on lists, but setting the queue
  mapping of the super-skb allows segments created by validation to
  inherit it.

- Call validate_xmit_skb_list() and transmit the resulting list. Each
  segment is passed with more = false, as the single skb was before, so
  drivers don't rely on a later call to flush their TX queue. The
  "again" result, which only xfrm offload sets, is ignored, as in
  __dev_direct_xmit().

Drops on either check now also fire trace_xdp_exception() with the
caller's action, like the other drops in generic_xdp_tx(). Network taps
are still bypassed, as before.

Invoking validation also unavoidably changes what happens to VLAN tags
that the ingress device placed into the hwaccel area. Generic XDP runs
in __netif_receive_skb_core() before the VLAN tag is handled, so the
hwaccel tag is still there in generic_xdp_tx(), outside the packet data
the XDP program saw. Egress devices without VLAN TX offload will ignore
it, and the frame goes out untagged. Now validate_xmit_vlan() inserts
the tag into the frame on such devices, as dev_queue_xmit() does, so the
frame is tagged whatever the egress device. This should be the more
correct behaviour, but I only consider it a positive side effect and not
itself worthy of a dedicated Fixes: tag.

Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 6d5fc1957989 ("xdp: Fix handling of devmap in generic XDP")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/core/dev.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 88860185f583..e98d85a53991 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5855,23 +5855,44 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog,
 		    u32 act)
 {
 	struct net_device *dev = skb->dev;
+	enum skb_drop_reason reason;
 	struct netdev_queue *txq;
-	bool free_skb = true;
+	bool again = false;
 	int cpu, rc;
 
+	reason = qdisc_pkt_len_segs_init(skb);
+	if (unlikely(reason)) {
+		trace_xdp_exception(dev, xdp_prog, act);
+		dev_core_stats_tx_dropped_inc(dev);
+		kfree_skb_reason(skb, reason);
+		return;
+	}
+
 	txq = netdev_core_pick_tx(dev, skb, NULL);
+	skb = validate_xmit_skb_list(skb, dev, &again);
+	if (!skb) {
+		trace_xdp_exception(dev, xdp_prog, act);
+		return;
+	}
+
 	cpu = smp_processor_id();
 	HARD_TX_LOCK(dev, txq, cpu);
-	if (!netif_xmit_frozen_or_drv_stopped(txq)) {
-		rc = netdev_start_xmit(skb, dev, txq, 0);
-		if (dev_xmit_complete(rc))
-			free_skb = false;
+	while (skb && !netif_xmit_frozen_or_drv_stopped(txq)) {
+		struct sk_buff *next = skb->next;
+
+		skb_mark_not_on_list(skb);
+		rc = netdev_start_xmit(skb, dev, txq, false);
+		if (!dev_xmit_complete(rc)) {
+			skb->next = next;
+			break;
+		}
+		skb = next;
 	}
 	HARD_TX_UNLOCK(dev, txq);
-	if (free_skb) {
+	if (skb) {
 		trace_xdp_exception(dev, xdp_prog, act);
 		dev_core_stats_tx_dropped_inc(dev);
-		kfree_skb(skb);
+		kfree_skb_list(skb);
 	}
 }
 
-- 
2.43.0

Re: [PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()

[Vladimir Oltean]

On Sat, Oct 31, 2026 at 11:48:09AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] net: out-of-bounds read in qdisc_pkt_len_segs_init() via generic_xdp_tx()
> - [High] net: shared skb->head corruption on cloned broadcast skbs in generic_xdp_tx()
> - [Medium] net: WARN_ONCE in skb_checksum_help() from generic_xdp_tx() after XDP resize
> --
> 
> commit 47b74c796cb5f05688236335aeb274a972cf3b13
> Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> net: validate xmit skbs in generic_xdp_tx()
> 
> This commit validates skbs in generic_xdp_tx() using
> qdisc_pkt_len_segs_init() and validate_xmit_skb_list() before passing them to
> netdev_start_xmit(). This ensures SKB_GSO_DODGY packets have gso_segs
> recomputed and unsupported offloads handled in software.
> 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 88860185f5832..e98d85a539916 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5855,23 +5855,44 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog,
> >  		    u32 act)
> >  {
> >  	struct net_device *dev = skb->dev;
> > +	enum skb_drop_reason reason;
> >  	struct netdev_queue *txq;
> > -	bool free_skb = true;
> > +	bool again = false;
> >  	int cpu, rc;
> >  
> > +	reason = qdisc_pkt_len_segs_init(skb);
> > +	if (unlikely(reason)) {
> > +		trace_xdp_exception(dev, xdp_prog, act);
> > +		dev_core_stats_tx_dropped_inc(dev);
> > +		kfree_skb_reason(skb, reason);
> > +		return;
> > +	}
> 
> [Severity: High]
> Can calling qdisc_pkt_len_segs_init() here cause an out-of-bounds read when a
> generic XDP program adjusts the packet head on a GSO packet?
> 
> In bpf_prog_run_generic_xdp(), when bpf_xdp_adjust_head() shifts xdp->data by
> a non-zero offset, skb->data, skb->mac_header, and skb->network_header are
> updated, but skb->transport_header (and skb->inner_transport_header) is left
> at its pre-adjustment offset:
> 
> net/core/dev.c:bpf_prog_run_generic_xdp() {
> 	...
> 	off = xdp->data - orig_data;
> 	if (off) {
> 		if (off > 0)
> 			__skb_pull(skb, off);
> 		else if (off < 0)
> 			__skb_push(skb, -off);
> 
> 		skb->mac_header += off;
> 		skb_reset_network_header(skb);
> 	}
> 	...
> }
> 
> When the XDP program returns XDP_TX or XDP_REDIRECT, generic_xdp_tx() calls
> qdisc_pkt_len_segs_init(skb), which assigns the signed int return value of
> skb_transport_offset(skb) (or skb_inner_transport_offset(skb)) to unsigned
> int hdr_len:
> 
> net/core/dev.c:qdisc_pkt_len_segs_init() {
> 	...
> 	if (!skb->encapsulation) {
> 		if (unlikely(!skb_transport_header_was_set(skb)))
> 			return SKB_NOT_DROPPED_YET;
> 		hdr_len = skb_transport_offset(skb);
> 	} else {
> 		hdr_len = skb_inner_transport_offset(skb);
> 	}
> 	/* + transport layer */
> 	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
> 		const struct tcphdr *th;
> 
> 		if (!pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
> 			return SKB_DROP_REASON_SKB_BAD_GSO;
> 
> 		th = (const struct tcphdr *)(skb->data + hdr_len);
> 		tlen = __tcp_hdrlen(th);
> 	...
> }
> 
> If bpf_xdp_adjust_head() pulled skb->data 1 to 20 bytes past the original
> skb_transport_header(skb) on a TCP GSO packet (or 1 to 8 bytes on a
> SKB_GSO_UDP_L4 packet), skb_transport_offset(skb) is in [-20, -1] and hdr_len
> becomes 0xffffffecU..0xffffffffU. In pskb_may_pull(skb, hdr_len +
> sizeof(struct tcphdr)), the 32-bit unsigned addition wraps to 0..19 and
> succeeds if skb_headlen(skb) is at least that small wrapped length. Then
> skb->data + hdr_len zero-extends hdr_len to 64 bits on 64-bit platforms and
> __tcp_hdrlen(th) reads approximately 4 GiB past skb->data.
> 
> Even when skb_transport_offset(skb) remains non-negative after
> bpf_xdp_adjust_head(), qdisc_pkt_len_segs_init() computes hdr_len and
> shinfo->gso_segs from the stale transport_header offset, whereas
> validate_xmit_skb_list() -> __skb_gso_segment() -> inet_gso_segment() /
> ipv6_gso_segment() later resets skb->transport_header and tcp_gso_segment()
> recomputes skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss) without
> re-checking dev->gso_max_segs.

The stale transport headers seems to be all pre-existing issues, based on
the fact that Sashiko reported it on this other patch as well,
triggerable from bpf_skb_net_hdr_pop() there:
https://lore.kernel.org/netdev/179290180493.3134.5235549211177646736@kernel.org/

As Eric says, the broken skb_transport_header should be fixed at producer side,
so there is nothing really actionable for this patch, I think.

> > +
> >  	txq = netdev_core_pick_tx(dev, skb, NULL);
> > +	skb = validate_xmit_skb_list(skb, dev, &again);
> > +	if (!skb) {
> > +		trace_xdp_exception(dev, xdp_prog, act);
> > +		return;
> > +	}
> 
> [Severity: High]
> Could passing skb to validate_xmit_skb_list() corrupt shared skb->head memory
> when a generic XDP program redirects with BPF_F_BROADCAST through a DEVMAP or
> DEVMAP_HASH?
> 
> During broadcast redirects, dev_map_redirect_multi() calls
> dev_map_redirect_clone() for each destination except the last, which clones
> the packet with skb_clone(skb, GFP_ATOMIC). In dev_map_generic_redirect(),
> skb_copy() is only called when dst->xdp_prog is attached:
> 
> kernel/bpf/devmap.c:dev_map_generic_redirect() {
> 	...
> 	if (dst->xdp_prog && skb_cloned(skb)) {
> 		struct sk_buff *nskb;
> 
> 		nskb = skb_copy(skb, GFP_ATOMIC);
> 	...
> }
> 
> When dst->xdp_prog is NULL, the cloned skb reaches generic_xdp_tx() ->
> validate_xmit_skb_list() -> validate_xmit_skb() -> __skb_gso_segment(). In
> __skb_gso_segment(), skb_cow_head(skb, 0) is only called when
> skb_needs_check(skb, true) is true, so it is skipped whenever ip_summed is
> CHECKSUM_PARTIAL or CHECKSUM_UNNECESSARY:
> 
> net/core/gso.c:__skb_gso_segment() {
> 	...
> 	if (unlikely(skb_needs_check(skb, tx_path))) {
> 		int err;
> 
> 		/* We're going to init ->check field in TCP or UDP header */
> 		err = skb_cow_head(skb, 0);
> 		if (err < 0)
> 			return ERR_PTR(err);
> 	}
> 	...
> }
> 
> If the packet is an IPv6 UFO skb with ip_summed == CHECKSUM_PARTIAL,
> udp6_ufo_fragment() skips gso_pskb_expand_head() because generic XDP ensures
> at least XDP_PACKET_HEADROOM (256 bytes) of headroom, which is larger than
> tnl_hlen + sizeof(struct frag_hdr). It then modifies uh->check and *prevhdr
> and shifts the MAC and IPv6 headers 8 bytes left in place via memmove() inside
> the shared skb->head buffer:
> 
> net/ipv6/udp_offload.c:udp6_ufo_fragment() {
> 		if (skb->mac_header < (tnl_hlen + frag_hdr_sz)) {
> 			if (gso_pskb_expand_head(skb, tnl_hlen + frag_hdr_sz))
> 				goto out;
> 		}
> 		...
> 		packet_start = (u8 *) skb->head + SKB_GSO_CB(skb)->mac_offset;
> 		memmove(packet_start-frag_hdr_sz, packet_start, unfrag_len);
> }
> 
> Similarly, udp4_ufo_fragment(), tcp4_gso_segment(), and tcp6_gso_segment()
> overwrite the L4 checksum field in place when ip_summed ==
> CHECKSUM_UNNECESSARY. When dev_map_redirect_multi() continues to the remaining
> broadcast destinations using the original skb, its mac_header and
> network_header still reference the pre-shift offsets in the mutated skb->head
> buffer.

Hmm, the problem seems valid, but AFAIU, the problem that __skb_gso_segment()
does not unshare the skb every time it should should be fixed in
__skb_gso_segment() itself, nothing actionable for this change.

If anything, cloned GSO skbs can already come from bridge flooding and
will reach __skb_gso_segment() from __dev_queue_xmit(), so the problem
is pre-existing.

> [Severity: Medium]
> Can calling validate_xmit_skb_list() here also trigger WARN_ONCE() in
> skb_checksum_help() when a generic XDP program shrinks or pulls a
> CHECKSUM_PARTIAL packet?
> 
> In bpf_prog_run_generic_xdp(), adjusting the packet via bpf_xdp_adjust_head()
> or bpf_xdp_adjust_tail() updates skb->data, skb->tail, and skb->len without
> updating or resetting skb->ip_summed, csum_start, or csum_offset. If a
> non-GSO packet with ip_summed == CHECKSUM_PARTIAL has its head pulled past
> csum_start or its tail shrunk below csum_start + csum_offset +
> sizeof(__sum16), and generic_xdp_tx() transmits it on a device without
> checksum offload (such as XDP_TX on a tun device without TUN_F_CSUM),
> validate_xmit_skb() -> skb_csum_hwoffload_help() -> skb_checksum_help() hits:
> 
> net/core/dev.c:skb_checksum_help() {
> 	...
> 	offset = skb_checksum_start_offset(skb);
> 	ret = -EINVAL;
> 	if (unlikely(offset >= skb_headlen(skb))) {
> 		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
> 		WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",
> 			  offset, skb_headlen(skb));
> 		goto out;
> 	}
> 	csum = skb_checksum(skb, offset, skb->len - offset, 0);
> 
> 	offset += skb->csum_offset;
> 	if (unlikely(offset + sizeof(__sum16) > skb_headlen(skb))) {
> 		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
> 		WARN_ONCE(true, "offset+2 (%zu) > skb_headlen() (%u)\n",
> 			  offset + sizeof(__sum16), skb_headlen(skb));
> 		goto out;
> 	}
> 	...
> }
> 
> Since offset is unsigned int, a negative skb_checksum_start_offset(skb) wraps
> to a large unsigned value, and either out-of-bounds offset triggers skb_dump()
> and WARN_ONCE().

Correction: 'offset' is int, not unsigned int (although this doesn't
change the outcome, since skb_headlen() returns unsigned int, the
comparison is made on unsigned values regardless).

So skb_checksum_start_offset() returns the offset between skb->csum_start
and skb->data, and Sashiko says that if bpf_xdp_adjust_head() pulls
skb->data beyond skb->csum_start for a non-GSO packet, this offset would
become negative, it would trigger the WARN_ONCE() and the skb would be
dropped.

I think the expected alternative outcome would be to do something like
bpf_skb_adjust_room() -> bpf_skb_net_shrink() -> bpf_skb_net_hdr_pop() ->
bpf_skb_generic_pop() -> skb_postpull_rcsum() -> __skb_postpull_rcsum()
does, and convert CHECKSUM_PARTIAL to CHECKSUM_NONE.

In any case this is something that needs to be addressed in bpf_xdp_adjust_head()
and bpf_xdp_adjust_tail(), because I think the exact same thing can
already happen even without XDP_TX / XDP_REDIRECT, i.e. with XDP_PASS
plus some sort of forwarding to the same kind of egress device (bridging
or IP forwarding).

In short, I don't think this patch makes any of the above problems worse
than they already are.

Re: [PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 (2) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch adds a call to qdisc_pkt_len_segs_init() at the top
  of generic_xdp_tx() (net/core/dev.c:5863).

Pre-existing issues:
- [Medium] The commit message says qdisc_pkt_len_segs_init() must run
  before validate_xmit_skb().
- [Medium] The patch now runs generic XDP TX/REDIRECT skbs through
  validate_xmit_skb().

--

net: validate xmit skbs in generic_xdp_tx()

This commit makes generic_xdp_tx() call qdisc_pkt_len_segs_init() and
validate_xmit_skb_list() before handing the skb to the driver. As a result,
SKB_GSO_DODGY skbs get their gso_segs recomputed, and offloads the egress
device lacks are done in software. The resulting list is transmitted one
skb at a time under the TX lock, and any leftovers are freed with
kfree_skb_list().

> Generic XDP skips all of this. Instead of calling dev_queue_xmit() or
> dev_direct_xmit(), the XDP_TX and XDP_REDIRECT actions end up in
> generic_xdp_tx(), which calls netdev_start_xmit() directly.

[ ... ]

> - Call qdisc_pkt_len_segs_init() first. It recomputes gso_segs for
>   SKB_GSO_DODGY skbs and drops malformed ones. Without it,
>   validate_xmit_skb() would compare the untrusted gso_segs = 0 against
>   dev->gso_max_segs in gso_within_dev_limits(), and the check would
>   always pass.

[Severity: Medium]
This isn't a bug introduced by this patch, but does dev_direct_xmit() have
the same gso_max_segs problem described here?

__dev_direct_xmit() calls validate_xmit_skb_list() without calling
qdisc_pkt_len_segs_init() first:

net/core/dev.c:__dev_direct_xmit() {
    ...
	skb = validate_xmit_skb_list(skb, dev, &again);
	if (skb != orig_skb)
		goto drop;
    ...
}

AF_PACKET with PACKET_VNET_HDR builds SKB_GSO_DODGY skbs with gso_segs = 0
in packet_snd() and tpacket_snd(). setsockopt() doesn't prevent combining
PACKET_VNET_HDR with PACKET_QDISC_BYPASS, and with that option set,
packet_xmit() sends these skbs to dev_direct_xmit():

packet_snd()
  virtio_net_hdr_to_skb()        /* DODGY, gso_segs = 0 */
  packet_xmit()
    dev_direct_xmit()
      __dev_direct_xmit()
        validate_xmit_skb()
          gso_within_dev_limits()  /* 0 > gso_max_segs, passes */
          __skb_gso_segment()
            tcp_gso_segment()      /* sets gso_segs, returns NULL */
        netdev_start_xmit()

The skb isn't segmented, so the skb != orig_skb check does not drop it. It
reaches the driver with gso_segs above the limit the driver set with
netif_set_tso_max_segs(). Triggering this needs CAP_NET_RAW.

The commit message lists dev_direct_xmit() alongside dev_queue_xmit() as a
path that generic XDP skips, which suggests it is already validated. Is
that accurate? Should the commit message be reworded, or should
__dev_direct_xmit() get the same treatment?

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 88860185f5832..e98d85a539916 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5855,23 +5855,44 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog,
>  		    u32 act)
>  {
>  	struct net_device *dev = skb->dev;
> +	enum skb_drop_reason reason;
>  	struct netdev_queue *txq;
> -	bool free_skb = true;
> +	bool again = false;
>  	int cpu, rc;
>  
> +	reason = qdisc_pkt_len_segs_init(skb);

[Severity: High]
Can this read far outside the skb if the XDP program pulled the head past
the transport header?

bpf_prog_run_generic_xdp() handles bpf_xdp_adjust_head() by moving the MAC
and network headers. It does not touch transport_header:

net/core/dev.c:bpf_prog_run_generic_xdp() {
    ...
	off = xdp->data - orig_data;
	if (off) {
		if (off > 0)
			__skb_pull(skb, off);
		else if (off < 0)
			__skb_push(skb, -off);

		skb->mac_header += off;
		skb_reset_network_header(skb);
	}
    ...
}

For a tun IFF_VNET_HDR GSO packet with VIRTIO_NET_HDR_F_NEEDS_CSUM,
__virtio_net_hdr_to_skb() sets transport_header from the user-supplied
csum_start through skb_partial_csum_set(). The only lower bound is
nh_min_len (20 bytes past the MAC header), and for TCPV4 the IP protocol
isn't checked.

Take csum_start = 34 (the outer UDP header of a VXLAN frame). A decap
program then strips 50 bytes and returns XDP_TX or XDP_REDIRECT. A devmap
egress program could do the same. In that case skb_transport_offset() is
-16 when this runs:

net/core/dev.c:qdisc_pkt_len_segs_init() {
    ...
		hdr_len = skb_transport_offset(skb);
    ...
		if (!pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
			return SKB_DROP_REASON_SKB_BAD_GSO;

		th = (const struct tcphdr *)(skb->data + hdr_len);
		tlen = __tcp_hdrlen(th);
    ...
}

hdr_len is an unsigned int, so it becomes 0xFFFFFFF0.
skb_transport_header_was_set() is true, so there is no early return.

The size_t sum 0x100000004 is truncated to 4 by the unsigned int len
parameter of pskb_may_pull(), so the check passes. On 64-bit, th then
points about 4 GiB past skb->data, and __tcp_hdrlen() reads th->doff
through it.

Before this patch, generic_xdp_tx() never read transport_header. The rest
of validation would be fine here, because inet_gso_segment() resets the
transport header.

The same helper is already reachable through XDP_PASS plus tc ingress in
sch_handle_ingress(). This patch puts it on the XDP_TX/XDP_REDIRECT path,
which needs no tc setup. As the commit message notes, an unprivileged
process can write the packet to a persistent TAP whose owner was set with
TUNSETOWNER.

Should generic_xdp_tx() reject or reset a negative skb_transport_offset()
before calling qdisc_pkt_len_segs_init()? Alternatively, should
qdisc_pkt_len_segs_init() handle that case itself?

[ ... ]

>  	txq = netdev_core_pick_tx(dev, skb, NULL);
> +	skb = validate_xmit_skb_list(skb, dev, &again);

[Severity: Medium]
This is a pre-existing issue, but can this call now hit the WARN_ONCE() in
skb_checksum_help() for CHECKSUM_PARTIAL skbs that the XDP program resized?

For tun skbs, csum_start and csum_offset come from user space through
virtio_net_hdr_to_skb()->skb_partial_csum_set(). They are only checked
against the layout before the XDP program runs.
bpf_prog_run_generic_xdp() changes skb->data and skb->len for
bpf_xdp_adjust_head() and bpf_xdp_adjust_tail(), but never rechecks the
CHECKSUM_PARTIAL state.

Take a non-GSO skb sent to a device without the matching checksum feature.
Examples are XDP_TX on a tun device where user space never enabled
TUN_F_CSUM with TUNSETOFFLOAD, or XDP_REDIRECT to a NIC without the
matching csum offload. The call chain is:

generic_xdp_tx()
  validate_xmit_skb_list()
    validate_xmit_skb()
      skb_csum_hwoffload_help()
        skb_checksum_help()

net/core/dev.c:skb_checksum_help() {
    ...
	offset = skb_checksum_start_offset(skb);
	ret = -EINVAL;
	if (unlikely(offset >= skb_headlen(skb))) {
		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
		WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",
			  offset, skb_headlen(skb));
		goto out;
	}
    ...
	offset += skb->csum_offset;
	if (unlikely(offset + sizeof(__sum16) > skb_headlen(skb))) {
		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
		WARN_ONCE(true, "offset+2 (%zu) > skb_headlen() (%u)\n",
    ...
}

If the program pulled the head past csum_start, offset is negative. It
compares as a large unsigned value against skb_headlen(), so the first
WARN_ONCE() fires. If the tail was shrunk below csum_start + csum_offset
+ 2, the second one fires.

The packet is dropped, and with panic_on_warn the kernel panics. The commit
message says validation fixes checksum offload by checksumming in software.
For packets the program modified this way, though, the WARN fires instead.

The stale checksum metadata comes from bpf_prog_run_generic_xdp() and
predates this patch. The same WARN can already be reached after XDP_PASS
when the skb is bridged or forwarded through dev_queue_xmit() to such a
device. This patch adds another way to reach it.

> +	if (!skb) {
> +		trace_xdp_exception(dev, xdp_prog, act);
> +		return;
> +	}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261030114753.185281-1-vladimir.oltean%40nxp.com

Re: [PATCH net 1/2] net: pass the XDP action to generic_xdp_tx() for tracing

[Simon Horman]

On Fri, Oct 30, 2026 at 01:47:52PM +0200, Vladimir Oltean wrote:
> When generic_xdp_tx() fails to transmit an skb, it reports the drop
> through trace_xdp_exception() with the action hardcoded to XDP_TX.
> That was right when generic XDP was added, and XDP_TX was the only
> caller. Since then, two XDP_REDIRECT paths have started to transmit
> through generic_xdp_tx() as well:
> 
> - xdp_do_generic_redirect(), for a redirect to an ifindex
> - dev_map_generic_redirect(), for DEVMAP and DEVMAP_HASH redirects,
>   including broadcast
> 
> Drops on those paths are reported as XDP_TX, an action which the
> program never returned. Tools which count xdp:xdp_exception events per
> action, such as the samples/bpf programs built on xdp_sample, therefore
> attribute generic XDP redirect drops to the wrong action. Drivers with
> native XDP already report XDP_TX and XDP_REDIRECT drops separately.
> 
> Add an "act" argument to generic_xdp_tx(), and have each caller pass
> the action that led to the transmission: XDP_TX from do_xdp_generic(),
> XDP_REDIRECT from the two redirect paths. generic_xdp_tx() is not
> exported, so these are all of its users.
> 
> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> Fixes: 6d5fc1957989 ("xdp: Fix handling of devmap in generic XDP")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()

[Simon Horman]

On Tue, Nov 03, 2026 at 01:39:26PM +0200, Vladimir Oltean wrote:
> On Sat, Oct 31, 2026 at 11:48:09AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> > - [High] net: out-of-bounds read in qdisc_pkt_len_segs_init() via generic_xdp_tx()
> > - [High] net: shared skb->head corruption on cloned broadcast skbs in generic_xdp_tx()
> > - [Medium] net: WARN_ONCE in skb_checksum_help() from generic_xdp_tx() after XDP resize

...

> In short, I don't think this patch makes any of the above problems worse
> than they already are.

I also believe that response can be extened to the overlapping feedback
provided by netdev-bot+sashiko@kernel.org 

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()

[Vladimir Oltean]

On Thu, Nov 05, 2026 at 01:31:08PM +0000, Simon Horman wrote:
> On Tue, Nov 03, 2026 at 01:39:26PM +0200, Vladimir Oltean wrote:
> > On Sat, Oct 31, 2026 at 11:48:09AM +0000, sashiko-bot@kernel.org wrote:
> > > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> > > - [High] net: out-of-bounds read in qdisc_pkt_len_segs_init() via generic_xdp_tx()
> > > - [High] net: shared skb->head corruption on cloned broadcast skbs in generic_xdp_tx()
> > > - [Medium] net: WARN_ONCE in skb_checksum_help() from generic_xdp_tx() after XDP resize
> 
> ...
> 
> > In short, I don't think this patch makes any of the above problems worse
> > than they already are.
> 
> I also believe that response can be extened to the overlapping feedback
> provided by netdev-bot+sashiko@kernel.org 
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

I saw the other Sashiko feedback, but I won't have time this week to see
whether all feedback is overlapping or whether there are some new points
which should result in a respin.

Re: [PATCH net 1/2] net: pass the XDP action to generic_xdp_tx() for tracing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 30 Oct 2026 13:47:52 +0200 you wrote:
> When generic_xdp_tx() fails to transmit an skb, it reports the drop
> through trace_xdp_exception() with the action hardcoded to XDP_TX.
> That was right when generic XDP was added, and XDP_TX was the only
> caller. Since then, two XDP_REDIRECT paths have started to transmit
> through generic_xdp_tx() as well:
> 
> - xdp_do_generic_redirect(), for a redirect to an ifindex
> - dev_map_generic_redirect(), for DEVMAP and DEVMAP_HASH redirects,
>   including broadcast
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: pass the XDP action to generic_xdp_tx() for tracing
    https://git.kernel.org/netdev/net-next/c/655978fc71fa
  - [net,2/2] net: validate xmit skbs in generic_xdp_tx()
    https://git.kernel.org/netdev/net-next/c/67b0cca9ba88

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html