netdev - Re: [PATCH net v4 1/2] net: gro: fix udp bad offset in socket lookup by adding {inner_}network_offset to napi_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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ada57e67645c7ebbbebe0c5d6e603113fc1be2e6.camel@redhat.com>
Date: Thu, 02 May 2024 11:18:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>, Richard Gobert
	 <richardbgobert@...il.com>
Cc: davem@...emloft.net, kuba@...nel.org, dsahern@...nel.org, 
	willemdebruijn.kernel@...il.com, alobakin@...me, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v4 1/2] net: gro: fix udp bad offset in socket
 lookup by adding {inner_}network_offset to napi_gro_cb

On Tue, 2024-04-30 at 18:17 +0200, Eric Dumazet wrote:
> On Tue, Apr 30, 2024 at 4:40 PM Richard Gobert <richardbgobert@...il.com> wrote:
> > 
> > Commits a602456 ("udp: Add GRO functions to UDP socket") and 57c67ff ("udp:
> > additional GRO support") introduce incorrect usage of {ip,ipv6}_hdr in the
> > complete phase of gro. The functions always return skb->network_header,
> > which in the case of encapsulated packets at the gro complete phase, is
> > always set to the innermost L3 of the packet. That means that calling
> > {ip,ipv6}_hdr for skbs which completed the GRO receive phase (both in
> > gro_list and *_gro_complete) when parsing an encapsulated packet's _outer_
> > L3/L4 may return an unexpected value.
> > 
> > This incorrect usage leads to a bug in GRO's UDP socket lookup.
> > udp{4,6}_lib_lookup_skb functions use ip_hdr/ipv6_hdr respectively. These
> > *_hdr functions return network_header which will point to the innermost L3,
> > resulting in the wrong offset being used in __udp{4,6}_lib_lookup with
> > encapsulated packets.
> > 
> > This patch adds network_offset and inner_network_offset to napi_gro_cb, and
> > makes sure both are set correctly.
> > 
> > To fix the issue, network_offsets union is used inside napi_gro_cb, in
> > which both the outer and the inner network offsets are saved.
> > 
> > Reproduction example:
> > 
> > Endpoint configuration example (fou + local address bind)
> > 
> >     # ip fou add port 6666 ipproto 4
> >     # ip link add name tun1 type ipip remote 2.2.2.1 local 2.2.2.2 encap fou encap-dport 5555 encap-sport 6666 mode ipip
> >     # ip link set tun1 up
> >     # ip a add 1.1.1.2/24 dev tun1
> > 
> > Netperf TCP_STREAM result on net-next before patch is applied:
> > 
> > net-next main, GRO enabled:
> >     $ netperf -H 1.1.1.2 -t TCP_STREAM -l 5
> >     Recv   Send    Send
> >     Socket Socket  Message  Elapsed
> >     Size   Size    Size     Time     Throughput
> >     bytes  bytes   bytes    secs.    10^6bits/sec
> > 
> >     131072  16384  16384    5.28        2.37
> > 
> > net-next main, GRO disabled:
> >     $ netperf -H 1.1.1.2 -t TCP_STREAM -l 5
> >     Recv   Send    Send
> >     Socket Socket  Message  Elapsed
> >     Size   Size    Size     Time     Throughput
> >     bytes  bytes   bytes    secs.    10^6bits/sec
> > 
> >     131072  16384  16384    5.01     2745.06
> > 
> > patch applied, GRO enabled:
> >     $ netperf -H 1.1.1.2 -t TCP_STREAM -l 5
> >     Recv   Send    Send
> >     Socket Socket  Message  Elapsed
> >     Size   Size    Size     Time     Throughput
> >     bytes  bytes   bytes    secs.    10^6bits/sec
> > 
> >     131072  16384  16384    5.01     2877.38
> > 
> > Fixes: 57c67ff4bd92 ("udp: additional GRO support")
> 
> Nit: I would think the bug was added later in
> a6024562ffd7 ("udp: Add GRO functions to UDP socket")

No need to repost: I'm updating that in PW.

Cheers,

Paolo