linux-kernel - Re: [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT

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: <CANn89iLi-NC=4jbNfFW7DELtHS2_JNAHiwRs7GbfZP2E9rGqXA@mail.gmail.com>
Date: Mon, 24 Feb 2025 14:00:46 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Felix Fietkau <nbd@....name>
Cc: netdev@...r.kernel.org, Neal Cardwell <ncardwell@...gle.com>, 
	Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller" <davem@...emloft.net>, 
	David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Simon Horman <horms@...nel.org>, Willem de Bruijn <willemb@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: ipv6: fix TCP GSO segmentation with NAT

On Mon, Feb 24, 2025 at 12:21 PM Felix Fietkau <nbd@....name> wrote:
>
> When updating the source/destination address, the TCP/UDP checksum needs to
> be updated as well.
>
> Fixes: bee88cd5bd83 ("net: add support for segmenting TCP fraglist GSO packets")
> Signed-off-by: Felix Fietkau <nbd@....name>
> ---
>  net/ipv6/tcpv6_offload.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index a45bf17cb2a1..5d0fcdbf57a1 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -113,24 +113,36 @@ static struct sk_buff *__tcpv6_gso_segment_list_csum(struct sk_buff *segs)
>         struct sk_buff *seg;
>         struct tcphdr *th2;
>         struct ipv6hdr *iph2;
> +       bool addr_equal;
>
>         seg = segs;
>         th = tcp_hdr(seg);
>         iph = ipv6_hdr(seg);
>         th2 = tcp_hdr(seg->next);
>         iph2 = ipv6_hdr(seg->next);
> +       addr_equal = ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
> +                    ipv6_addr_equal(&iph->daddr, &iph2->daddr);
>
>         if (!(*(const u32 *)&th->source ^ *(const u32 *)&th2->source) &&
> -           ipv6_addr_equal(&iph->saddr, &iph2->saddr) &&
> -           ipv6_addr_equal(&iph->daddr, &iph2->daddr))
> +           addr_equal)
>                 return segs;
>
>         while ((seg = seg->next)) {
>                 th2 = tcp_hdr(seg);
>                 iph2 = ipv6_hdr(seg);
>
> -               iph2->saddr = iph->saddr;
> -               iph2->daddr = iph->daddr;
> +               if (!addr_equal) {
> +                       inet_proto_csum_replace16(&th2->check, seg,
> +                                                 iph2->saddr.s6_addr32,
> +                                                 iph->saddr.s6_addr32,
> +                                                 true);
> +                       inet_proto_csum_replace16(&th2->check, seg,
> +                                                 iph2->daddr.s6_addr32,
> +                                                 iph->daddr.s6_addr32,
> +                                                 true);
> +                       iph2->saddr = iph->saddr;
> +                       iph2->daddr = iph->daddr;
> +               }
>                 __tcpv6_gso_segment_csum(seg, &th2->source, th->source);
>                 __tcpv6_gso_segment_csum(seg, &th2->dest, th->dest);

Good catch !

I note that __tcpv4_gso_segment_csum() does the needed csum changes
for both ports and address components.

Have you considered using the same logic for IPv6, to keep
__tcpv6_gso_segment_list_csum()
and __tcpv4_gso_segment_list_csum() similar ?