[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65f5a72e62658_6ef3e294dd@willemb.c.googlers.com.notmuch>
Date: Sat, 16 Mar 2024 10:05:34 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Antoine Tenart <atenart@...nel.org>,
davem@...emloft.net,
kuba@...nel.org,
pabeni@...hat.com,
edumazet@...gle.com
Cc: Antoine Tenart <atenart@...nel.org>,
steffen.klassert@...unet.com,
netdev@...r.kernel.org
Subject: Re: [PATCH net 1/4] udp: do not accept non-tunnel GSO skbs landing in
a tunnel
Antoine Tenart wrote:
> When rx-udp-gro-forwarding is enabled UDP packets might be GROed when
> being forwarded. If such packets might land in a tunnel this can cause
> various issues and udp_gro_receive makes sure this isn't the case by
> looking for a matching socket. This is performed in
> udp4/6_gro_lookup_skb but only in the current netns. This is an issue
> with tunneled packets when the endpoint is in another netns. In such
> cases the packets will be GROed at the UDP level, which leads to various
> issues later on. The same thing can happen with rx-gro-list.
>
> We saw this with geneve packets being GROed at the UDP level. In such
> case gso_size is set; later the packet goes through the geneve rx path,
> the geneve header is pulled, the offset are adjusted and frag_list skbs
> are not adjusted with regard to geneve. When those skbs hit
> skb_fragment, it will misbehave. Different outcomes are possible
> depending on what the GROed skbs look like; from corrupted packets to
> kernel crashes.
>
> One example is a BUG_ON[1] triggered in skb_segment while processing the
> frag_list. Because gso_size is wrong (geneve header was pulled)
> skb_segment thinks there is "geneve header size" of data in frag_list,
> although it's in fact the next packet. The BUG_ON itself has nothing to
> do with the issue. This is only one of the potential issues.
>
> Looking up for a matching socket in udp_gro_receive is fragile: the
> lookup could be extended to all netns (not speaking about performances)
> but nothing prevents those packets from being modified in between and we
> could still not find a matching socket. It's OK to keep the current
> logic there as it should cover most cases but we also need to make sure
> we handle tunnel packets being GROed too early.
>
> This is done by extending the checks in udp_unexpected_gso: GSO packets
> lacking the SKB_GSO_UDP_TUNNEL/_CUSM bits and landing in a tunnel must
> be segmented.
>
> [1] kernel BUG at net/core/skbuff.c:4408!
> RIP: 0010:skb_segment+0xd2a/0xf70
> __udp_gso_segment+0xaa/0x560
>
> Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.")
> Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets")
> Signed-off-by: Antoine Tenart <atenart@...nel.org>
Reviewed-by: Willem de Bruijn <willemb@...gle.com>
Powered by blists - more mailing lists