linux-kernel - Re: [PATCH net] net: fix segmentation after TCP/UDP fraglist GRO

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: <68716b45c8b0d_1682652947c@willemb.c.googlers.com.notmuch>
Date: Fri, 11 Jul 2025 15:51:33 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Felix Fietkau <nbd@....name>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 netdev@...r.kernel.org, 
 Eric Dumazet <edumazet@...gle.com>, 
 Neal Cardwell <ncardwell@...gle.com>, 
 Kuniyuki Iwashima <kuniyu@...gle.com>, 
 "David S. Miller" <davem@...emloft.net>, 
 David Ahern <dsahern@...nel.org>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Simon Horman <horms@...nel.org>, 
 Willem de Bruijn <willemb@...gle.com>, 
 Richard Gobert <richardbgobert@...il.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: fix segmentation after TCP/UDP fraglist GRO

Felix Fietkau wrote:
> On 06.07.25 15:45, Willem de Bruijn wrote:
> > Felix Fietkau wrote:
> >> Since "net: gro: use cb instead of skb->network_header", the skb network
> >> header is no longer set in the GRO path.
> >> This breaks fraglist segmentation, which relies on ip_hdr()/tcp_hdr()
> > 
> > Only ip_hdr is in scope.
> > 
> > Reviewing TCP and UDP GSO, tcp_hdr/transport header is used also
> > outside segment list. Non segment list GSO also uses ip_hdr in case
> > pseudo checksum needs to be set.
> Will change that in v2, thanks.
> > The GSO code is called with skb->data at the relevant header, so L4
> > helpers are not strictly needed. The main issue is that data will be
> > at the L4 header, and some GSO code also needs to see the IP header
> > (e.g., for aforementioned pseudo checksum calculation).
> > 
> >> to check for address/port changes.
> > 
> > If in GSO, then the headers are probably more correctly set at the end
> > of GRO, in gro_complete.
> 
> Just to clarify, in inet/ipv6_gro_complete you want me to iterate over 
> all fragment skbs, calculate the header offset based on the first skb, 
> and set it?

If that is the best way to fix this without causing regressions.

There may be a better solution. I just don't have a good suggestion
off the top of my head.

The blamed commit itself fixed an issue, where GRO code incorrectly
used the network header in GRO complete when it should be using the
inner network header.

Perhaps with moving that access to the CB, it is still safe to also
set the original network header. Perhaps Richard has an opinion.

If we want to be exact, these should still be updated to the inner
fields for encapsulated inner L4 protocols. Similar to what
tcp_gro_complete does for the transport header.