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 for Android: free password hash cracker in your pocket

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

Message-ID: <68307d87d58f8_180c7829432@willemb.c.googlers.com.notmuch>
Date: Fri, 23 May 2025 09:52:07 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, 
 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

Jakub Kicinski wrote:
> 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.

Most of this quotes my earlier patch, but without saying so.

https://lore.kernel.org/netdev/20241001171752.107580-1-willemdebruijn.kernel@gmail.com/

Btw, my email address was misspelled.

> 
> 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.