| Message ID | 20240315151722.119628-2-atenart@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | gro: various fixes related to UDP tunnels | expand |
Hi Antoine, kernel test robot noticed the following build errors: [auto build test ERROR on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Antoine-Tenart/udp-do-not-accept-non-tunnel-GSO-skbs-landing-in-a-tunnel/20240315-232048 base: net/main patch link: https://lore.kernel.org/r/20240315151722.119628-2-atenart%40kernel.org patch subject: [PATCH net 1/4] udp: do not accept non-tunnel GSO skbs landing in a tunnel config: arc-defconfig (https://download.01.org/0day-ci/archive/20240316/202403160519.XghWVi81-lkp@intel.com/config) compiler: arc-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160519.XghWVi81-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403160519.XghWVi81-lkp@intel.com/ All errors (new ones prefixed by >>): arc-elf-ld: net/ipv4/udp.o: in function `udp_queue_rcv_skb': >> udp.c:(.text+0x3aca): undefined reference to `udpv6_encap_needed_key' >> arc-elf-ld: udp.c:(.text+0x3aca): undefined reference to `udpv6_encap_needed_key'
Hi Antoine,
kernel test robot noticed the following build errors:
[auto build test ERROR on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Antoine-Tenart/udp-do-not-accept-non-tunnel-GSO-skbs-landing-in-a-tunnel/20240315-232048
base: net/main
patch link: https://lore.kernel.org/r/20240315151722.119628-2-atenart%40kernel.org
patch subject: [PATCH net 1/4] udp: do not accept non-tunnel GSO skbs landing in a tunnel
config: nios2-defconfig (https://download.01.org/0day-ci/archive/20240316/202403160550.1TZ0mDSX-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160550.1TZ0mDSX-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403160550.1TZ0mDSX-lkp@intel.com/
All errors (new ones prefixed by >>):
nios2-linux-ld: net/ipv4/udp.o: in function `raw_atomic_read':
include/linux/atomic/atomic-arch-fallback.h:457:(.text+0x4b28): undefined reference to `udpv6_encap_needed_key'
>> nios2-linux-ld: include/linux/atomic/atomic-arch-fallback.h:457:(.text+0x4b2c): undefined reference to `udpv6_encap_needed_key'
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@kernel.org> Reviewed-by: Willem de Bruijn <willemb@google.com>
Quoting kernel test robot (2024-03-15 22:21:50) > Hi Antoine, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on net/main] > > url: https://github.com/intel-lab-lkp/linux/commits/Antoine-Tenart/udp-do-not-accept-non-tunnel-GSO-skbs-landing-in-a-tunnel/20240315-232048 > base: net/main > patch link: https://lore.kernel.org/r/20240315151722.119628-2-atenart%40kernel.org > patch subject: [PATCH net 1/4] udp: do not accept non-tunnel GSO skbs landing in a tunnel > config: arc-defconfig (https://download.01.org/0day-ci/archive/20240316/202403160519.XghWVi81-lkp@intel.com/config) > compiler: arc-elf-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160519.XghWVi81-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202403160519.XghWVi81-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > arc-elf-ld: net/ipv4/udp.o: in function `udp_queue_rcv_skb': > >> udp.c:(.text+0x3aca): undefined reference to `udpv6_encap_needed_key' > >> arc-elf-ld: udp.c:(.text+0x3aca): undefined reference to `udpv6_encap_needed_key' Issue is with CONFIG_IPV6=n. The following should fix it, diff --git a/include/linux/udp.h b/include/linux/udp.h index 51558d6527f0..05231fff8703 100644 --- a/include/linux/udp.h +++ b/include/linux/udp.h @@ -151,7 +151,22 @@ static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk, } DECLARE_STATIC_KEY_FALSE(udp_encap_needed_key); +#if IS_ENABLED(CONFIG_IPV6) DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key); +#endif + +static inline bool udp_encap_needed(void) +{ + if (static_branch_unlikely(&udp_encap_needed_key)) + return true; + +#if IS_ENABLED(CONFIG_IPV6) + if (static_branch_unlikely(&udpv6_encap_needed_key)) + return true; +#endif + + return false; +} static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb) { @@ -170,8 +185,7 @@ static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb) * land in a tunnel as the socket check in udp_gro_receive cannot be * foolproof. */ - if ((static_branch_unlikely(&udp_encap_needed_key) || - static_branch_unlikely(&udpv6_encap_needed_key)) && + if (udp_encap_needed() && READ_ONCE(udp_sk(sk)->encap_rcv) && !(skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_UDP_TUNNEL_CSUM)))
diff --git a/include/linux/udp.h b/include/linux/udp.h index 3748e82b627b..51558d6527f0 100644 --- a/include/linux/udp.h +++ b/include/linux/udp.h @@ -150,6 +150,9 @@ static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk, } } +DECLARE_STATIC_KEY_FALSE(udp_encap_needed_key); +DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key); + static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb) { if (!skb_is_gso(skb)) @@ -163,6 +166,17 @@ static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb) !udp_test_bit(ACCEPT_FRAGLIST, sk)) return true; + /* GSO packets lacking the SKB_GSO_UDP_TUNNEL/_CUSM bits might still + * land in a tunnel as the socket check in udp_gro_receive cannot be + * foolproof. + */ + if ((static_branch_unlikely(&udp_encap_needed_key) || + static_branch_unlikely(&udpv6_encap_needed_key)) && + READ_ONCE(udp_sk(sk)->encap_rcv) && + !(skb_shinfo(skb)->gso_type & + (SKB_GSO_UDP_TUNNEL | SKB_GSO_UDP_TUNNEL_CSUM))) + return true; + return false; } diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index b9880743765c..e9719afe91cf 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -551,8 +551,10 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, unsigned int off = skb_gro_offset(skb); int flush = 1; - /* we can do L4 aggregation only if the packet can't land in a tunnel - * otherwise we could corrupt the inner stream + /* We can do L4 aggregation only if the packet can't land in a tunnel + * otherwise we could corrupt the inner stream. Detecting such packets + * cannot be foolproof and the aggregation might still happen in some + * cases. Such packets should be caught in udp_unexpected_gso later. */ NAPI_GRO_CB(skb)->is_flist = 0; if (!sk || !udp_sk(sk)->gro_receive) {
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@kernel.org> --- include/linux/udp.h | 14 ++++++++++++++ net/ipv4/udp_offload.c | 6 ++++-- 2 files changed, 18 insertions(+), 2 deletions(-)