Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: gso: always scan dodgy frag_list for linear heads
@ 2026-08-27 13:28 Guidong Han
  2026-08-27 13:44 ` Eric Dumazet
  0 siblings, 1 reply; 2+ messages in thread
From: Guidong Han @ 2026-08-27 13:28 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Willem de Bruijn, Shmulik Ladkani, Alexander Duyck,
	Cen Zhang, Guidong Han, stable

skb_segment() moves frag_list data into the segments either by sharing
page frags (zero-copy, requires the source memory to be page backed)
or by copying. Sharing into a linear kmalloc head is invalid, so for
SKB_GSO_DODGY skbs (untrusted GSO metadata, e.g. from a virtio net
header supplied by a VM guest or an AF_PACKET/TUN user) the frag_list
gets a defensive scan: if any member has a linear head without
head_frag set, NETIF_F_SG is cleared to force the copying fallback.
Without it, segmentation dies at BUG_ON(!list_skb->head_frag).

The scan is skipped when mss == skb_headlen(head_skb), on the
assumption that the frag_list members then terminate on exact mss
boundaries, so segments never cut into the middle of a member and
page sharing stays safe. That assumption comes from GRO-built lists,
where every member holds exactly one received segment. It does not
hold in general: the first segment also contains head_skb's paged
frags, so even with mss == skb_headlen(head_skb) a frag_list member
need not start on an mss boundary.

Such a layout is reachable from a guest: it sends two IPv4 fragments
through virtio-net, the first with a forged gso_size equal to the
reassembled linear head length, and conntrack defrag appends the
second, kmalloc-backed fragment to the frag_list. Forwarding then
forces software GSO (gso_size > egress MTU, or the egress lacks
NETIF_F_FRAGLIST) and panics the host: an unprivileged guest can take
down the host and all co-located VMs. The same layout is reachable
from host userspace via AF_PACKET PACKET_VNET_HDR or TUN with
IFF_VNET_HDR, given any conntrack user.

Drop the mss != skb_headlen(head_skb) test so the scan runs for all
dodgy frag_list skbs.

Fixes: 3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
Cc: stable@vger.kernel.org
Co-developed-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: Guidong Han <2045gemini@gmail.com>
---
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ba3dbac80fb4..e97b27ec13fd 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4796,7 +4796,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 	int nfrags, pos;
 
 	if ((skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY) &&
-	    mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb)) {
+	    mss != GSO_BY_FRAGS) {
 		struct sk_buff *check_skb;
 
 		for (check_skb = list_skb; check_skb; check_skb = check_skb->next) {
@@ -4804,11 +4804,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 				/* gso_size is untrusted, and we have a frag_list with
 				 * a linear non head_frag item.
 				 *
-				 * If head_skb's headlen does not fit requested gso_size,
-				 * it means that the frag_list members do NOT terminate
-				 * on exact gso_size boundaries. Hence we cannot perform
-				 * skb_frag_t page sharing. Therefore we must fallback to
-				 * copying the frag_list skbs; we do so by disabling SG.
+				 * The frag_list members cannot be trusted to terminate
+				 * on exact gso_size boundaries: head_skb's paged frags
+				 * take part in the first segment, so even
+				 * mss == skb_headlen(head_skb) does not guarantee the
+				 * alignment. Hence we cannot perform skb_frag_t page
+				 * sharing. Therefore we must fallback to copying the
+				 * frag_list skbs; we do so by disabling SG.
 				 */
 				features &= ~NETIF_F_SG;
 				break;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net: gso: always scan dodgy frag_list for linear heads
  2026-08-27 13:28 [PATCH net] net: gso: always scan dodgy frag_list for linear heads Guidong Han
@ 2026-08-27 13:44 ` Eric Dumazet
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Dumazet @ 2026-08-27 13:44 UTC (permalink / raw)
  To: Guidong Han, Xinyang Ge
  Cc: netdev, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Willem de Bruijn, Shmulik Ladkani, Alexander Duyck,
	Cen Zhang, stable

On Thu, Aug 27, 2026 at 3:28 PM Guidong Han <2045gemini@gmail.com> wrote:
>
> skb_segment() moves frag_list data into the segments either by sharing
> page frags (zero-copy, requires the source memory to be page backed)
> or by copying. Sharing into a linear kmalloc head is invalid, so for
> SKB_GSO_DODGY skbs (untrusted GSO metadata, e.g. from a virtio net
> header supplied by a VM guest or an AF_PACKET/TUN user) the frag_list
> gets a defensive scan: if any member has a linear head without
> head_frag set, NETIF_F_SG is cleared to force the copying fallback.
> Without it, segmentation dies at BUG_ON(!list_skb->head_frag).
>
> The scan is skipped when mss == skb_headlen(head_skb), on the
> assumption that the frag_list members then terminate on exact mss
> boundaries, so segments never cut into the middle of a member and
> page sharing stays safe. That assumption comes from GRO-built lists,
> where every member holds exactly one received segment. It does not
> hold in general: the first segment also contains head_skb's paged
> frags, so even with mss == skb_headlen(head_skb) a frag_list member
> need not start on an mss boundary.
>
> Such a layout is reachable from a guest: it sends two IPv4 fragments
> through virtio-net, the first with a forged gso_size equal to the
> reassembled linear head length, and conntrack defrag appends the
> second, kmalloc-backed fragment to the frag_list. Forwarding then
> forces software GSO (gso_size > egress MTU, or the egress lacks
> NETIF_F_FRAGLIST) and panics the host: an unprivileged guest can take
> down the host and all co-located VMs. The same layout is reachable
> from host userspace via AF_PACKET PACKET_VNET_HDR or TUN with
> IFF_VNET_HDR, given any conntrack user.
>
> Drop the mss != skb_headlen(head_skb) test so the scan runs for all
> dodgy frag_list skbs.
>
> Fixes: 3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
> Cc: stable@vger.kernel.org
> Co-developed-by: Cen Zhang <zzzccc427@gmail.com>
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> Signed-off-by: Guidong Han <2045gemini@gmail.com>
> ---
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index ba3dbac80fb4..e97b27ec13fd 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4796,7 +4796,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>         int nfrags, pos;
>
>         if ((skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY) &&
> -           mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb)) {
> +           mss != GSO_BY_FRAGS) {
>                 struct sk_buff *check_skb;
>
>                 for (check_skb = list_skb; check_skb; check_skb = check_skb->next) {
> @@ -4804,11 +4804,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>                                 /* gso_size is untrusted, and we have a frag_list with
>                                  * a linear non head_frag item.
>                                  *
> -                                * If head_skb's headlen does not fit requested gso_size,
> -                                * it means that the frag_list members do NOT terminate
> -                                * on exact gso_size boundaries. Hence we cannot perform
> -                                * skb_frag_t page sharing. Therefore we must fallback to
> -                                * copying the frag_list skbs; we do so by disabling SG.
> +                                * The frag_list members cannot be trusted to terminate
> +                                * on exact gso_size boundaries: head_skb's paged frags
> +                                * take part in the first segment, so even
> +                                * mss == skb_headlen(head_skb) does not guarantee the
> +                                * alignment. Hence we cannot perform skb_frag_t page
> +                                * sharing. Therefore we must fallback to copying the
> +                                * frag_list skbs; we do so by disabling SG.
>                                  */
>                                 features &= ~NETIF_F_SG;
>                                 break;

This is not enough.

We have a better fix coming from Xinyang Ge soon.

pw-bot: rejected

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index fc0cb993959f9b6506063a4118078abde12521d9..c17e57ec7d5cdb6aac4b26777ab96254940b1945
100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -435,6 +435,13 @@ int inet_frag_queue_insert(struct inet_frag_queue
*q, struct sk_buff *skb,
 {
        struct sk_buff *last = q->fragments_tail;

+       /* An IP fragment is never a GSO packet, but an untrusted source
+        * (virtio_net_hdr) may have attached GSO metadata to it. Do not let
+        * that reach the reassembled skb, whose head keeps the first
+        * fragment's shinfo and whose frag_list is not GRO-shaped.
+        */
+       skb_gso_reset(skb);
+
        /* RFC5722, Section 4, amended by Errata ID : 3089
         *                          When reassembling an IPv6 datagram, if
         *   one or more its constituent fragments is determined to be an

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-27 13:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 13:28 [PATCH net] net: gso: always scan dodgy frag_list for linear heads Guidong Han
2026-08-27 13:44 ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox