netdev - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250522094954.2f8090ce@kernel.org>
Date: Thu, 22 May 2025 09:49:54 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Shiming Cheng <shiming.cheng@...iatek.com>
Cc: <willemdebruijin.kernel@...il.com>, <edumazet@...gle.com>,
 <davem@...emloft.net>, <pabeni@...hat.com>, <matthias.bgg@...il.com>,
 <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
 <lena.wang@...iatek.com>
Subject: Re: [PATCH] net: fix udp gso skb_segment after pull from frag_list

On Thu, 22 May 2025 11:18:04 +0800 Shiming Cheng wrote:
>     Detect invalid geometry due to pull from frag_list, and pass to
>     regular skb_segment. if only part of the fraglist payload is pulled
>     into head_skb, When splitting packets in the skb_segment function,
>     it will always cause exception as 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.
> 
>     When splitting packets in the skb_segment function, the first two
>     packets of (11,11) bytes are split using skb_copy_bits. But when
>     the last packet of 10 bytes is split, because hsize becomes nagative,
>     it enters the skb_clone process instead of continuing to use
>     skb_copy_bits. In fact, the data for skb_clone has already been
>     copied into the second packet.
> 
>     when hsize < 0,  the payload of the fraglist has already been copied
>     (with skb_copy_bits), so there is no need to enter skb_clone to
>     process this packet. Instead, continue using skb_copy_bits to process
>     the next packet.

nit: please un-indent the text, you can keep the stack trace indented
but the commit message explanation should not be. And if you can
run the stack trace thru decode_stacktrace to add source code
references.

This patch seems to be causing regressions for SCTP, see:
https://lore.kernel.org/all/aC82JEehNShMjW8-@strlen.de/
If you send a v2 please make sure you add test cases to
net/core/net_test.c for both the geometry you're trying to fix,
and the geometry you got wrong in v1.