netdev - Re: [PATCH v2 net] net: skip offload for NETIF_F_IPV6

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: <CANn89iK1hdC3Nt8KPhOtTF8vCPc1AHDCtse_BTNki1pWxAByTQ@mail.gmail.com>
Date: Mon, 23 Dec 2024 11:40:10 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Benoît Monin <benoit.monin@....fr>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Jiri Pirko <jiri@...nulli.us>, 
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net] net: skip offload for NETIF_F_IPV6_CSUM if ipv6
 header contains extension

On Thu, Oct 24, 2024 at 4:01 PM Benoît Monin <benoit.monin@....fr> wrote:
>
> As documented in skbuff.h, devices with NETIF_F_IPV6_CSUM capability
> can only checksum TCP and UDP over IPv6 if the IP header does not
> contains extension.
>
> This is enforced for UDP packets emitted from user-space to an IPv6
> address as they go through ip6_make_skb(), which calls
> __ip6_append_data() where a check is done on the header size before
> setting CHECKSUM_PARTIAL.
>
> But the introduction of UDP encapsulation with fou6 added a code-path
> where it is possible to get an skb with a partial UDP checksum and an
> IPv6 header with extension:
> * fou6 adds a UDP header with a partial checksum if the inner packet
> does not contains a valid checksum.
> * ip6_tunnel adds an IPv6 header with a destination option extension
> header if encap_limit is non-zero (the default value is 4).
>
> The thread linked below describes in more details how to reproduce the
> problem with GRE-in-UDP tunnel.
>
> Add a check on the network header size in skb_csum_hwoffload_help() to
> make sure no IPv6 packet with extension header is handed to a network
> device with NETIF_F_IPV6_CSUM capability.
>
> Link: https://lore.kernel.org/netdev/26548921.1r3eYUQgxm@benoit.monin/T/#u
> Fixes: aa3463d65e7b ("fou: Add encap ops for IPv6 tunnels")
> Signed-off-by: Benoît Monin <benoit.monin@....fr>
> ---
> changelog
> * v2:
>     - patch against net instead of net-next
>     - clarify documentation of NETIF_F_IPV6_CSUM
>     - add link to thread describing the problem
>     - add fixes tag
>     - use vlan_get_protocol to check for IPv6
> * v1:
>     - https://lore.kernel.org/netdev/0dc0c2af98e96b1df20bd36aeaed4eb4e27d507e.1728056028.git.benoit.monin@gmx.fr/T/#u
> ---
>  net/core/dev.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ea5fbcd133ae..8453e14d301b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3639,6 +3639,9 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
>                 return 0;
>
>         if (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) {
> +               if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
> +                   skb_network_header_len(skb) != sizeof(struct ipv6hdr))
> +                       goto sw_checksum;
>                 switch (skb->csum_offset) {
>                 case offsetof(struct tcphdr, check):
>                 case offsetof(struct udphdr, check):
> @@ -3646,6 +3649,7 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
>                 }
>         }
>
> +sw_checksum:
>         return skb_checksum_help(skb);
>  }
>  EXPORT_SYMBOL(skb_csum_hwoffload_help);


FYI, this patch broke BIG TCP over IPv6.

[  239.698598] Oops skb_network_header_len()=48 skb->len=67210
[  239.704122] skb len=67210 headroom=162 headlen=94 tailroom=0
               mac=(162,14) mac_len=0 net=(176,48) trans=224
               shinfo(txflags=0 nr_frags=3 gso(size=1428 type=16 segs=47))
               csum(0x1000e0 start=224 offset=16 ip_summed=3
complete_sw=0 valid=0 level=0)
               hash(0xadf29e31 sw=0 l4=1) proto=0x86dd pkttype=0 iif=0
               priority=0x18020 mark=0x0 alloc_cpu=46 vlan_all=0x0
               encapsulation=0 inner(proto=0x0000, mac=0, net=0,
trans=0)\x00, net=0, trans=0)
[  239.704153] dev name=eth2 feat=0x0000030000114ab3
[  239.704155] sk family=10 type=1 proto=6
[  239.704156] skb linear:   00000000: 02 32 00 00 00 00 94 eb 2c 18
9c d8 86 dd 60 2d
[  239.704157] skb linear:   00000010: 31 9e 00 00 00 7f 20 02 0a 0d
87 01 00 00 00 00
[  239.704158] skb linear:   00000020: 00 00 00 00 00 00 20 02 0a 05
68 30 1f 86 00 00
[  239.704159] skb linear:   00000030: 00 00 00 00 00 00 06 00 c2 04
00 01 06 54 ac 4c
[  239.704160] skb linear:   00000040: 81 9b 82 a6 d6 74 ca 75 8d 24
80 18 00 42 69 21
[  239.704161] skb linear:   00000050: 00 00 01 01 08 0a 1b fe e2 2a ca 8f 78 6e
[  239.704162] skb frag:     00000000: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704163] skb frag:     00000010: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704163] skb frag:     00000020: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704164] skb frag:     00000030: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704165] skb frag:     00000040: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704166] skb frag:     00000050: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704166] skb frag:     00000060: 6e 65 74 70 65 72 66 00 6e 65
74 70 65 72 66 00
[  239.704167] skb frag:     00000070: 6e 65