| Message ID | 5fbeecfc311ea182aa1d1c771725ab8b4cac515e.1729778144.git.benoit.monin@gmx.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net] net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension | expand |
Benoît Monin 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@gmx.fr> Reviewed-by: Willem de Bruijn <willemb@google.com> > --- > 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; skb_network_header_len requires skb->transport_header to be set. This is not true for all egress packets. See for instance commit d2aa125d6290 ("net: Don't set transport offset to invalid value"). But it should be true for all CHECKSUM_PARTIAL packets. See for instance skb_partial_csum_set. So LGTM. Just calling this out as it is not obvious and in case someone does know a counter example of CHECKSUM_PARTIAL and !skb_transport_header_was_set.
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);
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@gmx.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(+)