netdev - RE: [PATCH net] tcp: gso: really support BIG TCP

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] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <MW5PR15MB512161EEAF0B6731DCA5AE80A449A@MW5PR15MB5121.namprd15.prod.outlook.com>
Date: Thu, 1 Jun 2023 21:46:50 +0000
From: Alexander Duyck <alexanderduyck@...a.com>
To: Eric Dumazet <edumazet@...gle.com>,
        "David S . Miller"
	<davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Xin Long
	<lucien.xin@...il.com>, David Ahern <dsahern@...nel.org>,
        "eric.dumazet@...il.com" <eric.dumazet@...il.com>
Subject: RE: [PATCH net] tcp: gso: really support BIG TCP



> -----Original Message-----
> From: Eric Dumazet <edumazet@...gle.com>
> Sent: Thursday, June 1, 2023 2:18 PM
> To: David S . Miller <davem@...emloft.net>; Jakub Kicinski
> <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>
> Cc: netdev@...r.kernel.org; Xin Long <lucien.xin@...il.com>; David Ahern
> <dsahern@...nel.org>; eric.dumazet@...il.com; Eric Dumazet
> <edumazet@...gle.com>; Alexander Duyck <alexanderduyck@...a.com>
> Subject: [PATCH net] tcp: gso: really support BIG TCP
> 
> > 
> We missed that tcp_gso_segment() was assuming skb->len was smaller than
> 65535 :
> 
> oldlen = (u16)~skb->len;
> 
> This part came with commit 0718bcc09b35 ("[NET]: Fix CHECKSUM_HW GSO
> problems.")
> 
> This leads to wrong TCP checksum.
> 
> Simply use csum_fold() to support 32bit packet lengthes.
> 
> oldlen name is a bit misleading, as it is the contribution of skb->len on the
> input skb TCP checksum. I added a comment to clarify this point.
>
> Fixes: 09f3d1a3a52c ("ipv6/gso: remove temporary HBH/jumbo header")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Alexander Duyck <alexanderduyck@...com>
> ---
>  net/ipv4/tcp_offload.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index
> 45dda788938704c3f762256266d9ea29b6ded4a5..5a1a163b2d859696df8f204b5
> 0e3fc76c14b64e9 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -75,7 +75,8 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
>  	if (!pskb_may_pull(skb, thlen))
>  		goto out;
> 
> -	oldlen = (u16)~skb->len;
> +	/* Contribution of skb->len in current TCP checksum */
> +	oldlen = (__force u32)csum_fold((__force __wsum)skb->len);
>  	__skb_pull(skb, thlen);
> 
>  	mss = skb_shinfo(skb)->gso_size;

The only thing I am not sure about with this change is if we risk overflowing a u32 with all the math that may occur. The original code couldn't exceed a u16 for delta since we were essentially adding -oldlen + new header + mss. With us now allowing the use of a value larger than 16 bit we should be able to have the resultant value exceed 16b which means we might overflow when we add it to the current checksum.

As such we may want to look at holding off on the csum_fold until after we have added the new header and mss and then store the folded value in delta.