* [PATCH net 2/2] net: validate xmit skbs in generic_xdp_tx()
2026-10-30 11:47 [PATCH net 1/2] net: pass the XDP action to generic_xdp_tx() for tracing Vladimir Oltean
@ 2026-10-30 11:47 ` Vladimir Oltean
2026-10-31 11:48 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Oltean @ 2026-10-30 11:47 UTC (permalink / raw)
To: netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Emil Tsalapatis, Ihor Solodrai, Kuniyuki Iwashima, Wang Zhan,
Martin Karsten, Toshiaki Makita, linux-kernel, bpf,
Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo,
virtualization, Willem de Bruijn
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
^ permalink raw reply related [flat|nested] 3+ messages in thread