linux-kernel - Re: [PATCH net v4] net: fix udp gso skb_segment after pull from frag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <683759284c3a9_38477c2943e@willemb.c.googlers.com.notmuch>
Date: Wed, 28 May 2025 14:42:48 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Shiming Cheng <shiming.cheng@...iatek.com>, 
 willemdebruijn.kernel@...il.com, 
 willemb@...gle.com, 
 edumazet@...gle.com, 
 davem@...emloft.net, 
 kuba@...nel.org, 
 pabeni@...hat.com, 
 matthias.bgg@...il.com
Cc: linux-kernel@...r.kernel.org, 
 netdev@...r.kernel.org, 
 shiming.cheng@...iatek.com, 
 lena.wang@...iatek.com
Subject: Re: [PATCH net v4] net: fix udp gso skb_segment after pull from
 frag_list

Shiming Cheng wrote:

Code LGTM thanks. Let's make sure that the commit message matches.

> Detect invalid geometry due to pull from frag_list, and pass to
> regular skb_segment. 

This is a leftover from copying from my patch, and does not really
capture what the commit does.

How about something like

Commit a1e40ac5b5e9 ("net: gso: fix udp gso fraglist segmentation
after pull from frag_list") detected invalid geometry in frag_list
skbs and redirects them from skb_segment_list to more robust
skb_segment.

But some packets with modified geometry can also hit bugs in that
code. We don't know how many such cases exist. Addressing each one by
one also requires touching the complex skb_segment code, which risks
introducing bugs for other types of skbs.

Instead, linearize all these packets that fail the basic invariants on
gso fraglist skbs. That is more robust.


> If only part of the fraglist payload is pulled
> into head_skb, it will always cause exception when splitting
> skbs by skb_segment. For detailed call stack information, see below.
> 
> Valid SKB_GSO_FRAGLIST skbs
> - consist of two or more segments
> - the head_skb holds the protocol headers plus first gso_size
> - one or more frag_list skbs hold exactly one segment
> - all but the last must be gso_size
> 
> Optional datapath hooks such as NAT and BPF (bpf_skb_pull_data) can
> modify fraglist skbs, breaking these invariants.
> 
> In extreme cases they pull one part of data into skb linear. For UDP,
> this  causes three payloads with lengths of (11,11,10) bytes were
> pulled tail to become (12,10,10) bytes.
> 
> The skbs no longer meets the above SKB_GSO_FRAGLIST conditions because
> payload was pulled into head_skb, it needs to be linearized before pass
> to regular skb_segment.
> 
>     skb_segment+0xcd0/0xd14
>     __udp_gso_segment+0x334/0x5f4
>     udp4_ufo_fragment+0x118/0x15c
>     inet_gso_segment+0x164/0x338
>     skb_mac_gso_segment+0xc4/0x13c
>     __skb_gso_segment+0xc4/0x124
>     validate_xmit_skb+0x9c/0x2c0
>     validate_xmit_skb_list+0x4c/0x80
>     sch_direct_xmit+0x70/0x404
>     __dev_queue_xmit+0x64c/0xe5c
>     neigh_resolve_output+0x178/0x1c4
>     ip_finish_output2+0x37c/0x47c
>     __ip_finish_output+0x194/0x240
>     ip_finish_output+0x20/0xf4
>     ip_output+0x100/0x1a0
>     NF_HOOK+0xc4/0x16c
>     ip_forward+0x314/0x32c
>     ip_rcv+0x90/0x118
>     __netif_receive_skb+0x74/0x124
>     process_backlog+0xe8/0x1a4
>     __napi_poll+0x5c/0x1f8
>     net_rx_action+0x154/0x314
>     handle_softirqs+0x154/0x4b8
> 
>     [118.376811] [C201134] rxq0_pus: [name:bug&]kernel BUG at net/core/skbuff.c:4278!
>     [118.376829] [C201134] rxq0_pus: [name:traps&]Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
>     [118.470774] [C201134] rxq0_pus: [name:mrdump&]Kernel Offset: 0x178cc00000 from 0xffffffc008000000
>     [118.470810] [C201134] rxq0_pus: [name:mrdump&]PHYS_OFFSET: 0x40000000
>     [118.470827] [C201134] rxq0_pus: [name:mrdump&]pstate: 60400005 (nZCv daif +PAN -UAO)
>     [118.470848] [C201134] rxq0_pus: [name:mrdump&]pc : [0xffffffd79598aefc] skb_segment+0xcd0/0xd14
>     [118.470900] [C201134] rxq0_pus: [name:mrdump&]lr : [0xffffffd79598a5e8] skb_segment+0x3bc/0xd14
>     [118.470928] [C201134] rxq0_pus: [name:mrdump&]sp : ffffffc008013770
> 
> Fixes: a1e40ac5b5e9 ("net: gso: fix udp gso fraglist segmentation after pull from frag_list")
> Signed-off-by: Shiming Cheng <shiming.cheng@...iatek.com>
> ---
>  net/ipv4/udp_offload.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index a5be6e4ed326..59ddb85c858c 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -273,6 +273,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  	bool copy_dtor;
>  	__sum16 check;
>  	__be16 newlen;
> +	int ret = 0;
>  
>  	mss = skb_shinfo(gso_skb)->gso_size;
>  	if (gso_skb->len <= sizeof(*uh) + mss)
> @@ -301,6 +302,10 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  		if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
>  			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
>  
> +		ret = __skb_linearize(gso_skb);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
>  		 /* Setup csum, as fraglist skips this in udp4_gro_receive. */
>  		gso_skb->csum_start = skb_transport_header(gso_skb) - gso_skb->head;
>  		gso_skb->csum_offset = offsetof(struct udphdr, check);
> -- 
> 2.45.2
>