netdev - Re: [PATCH v7 net-next 09/12] gro: prevent ACE field corruption & better AccECN handling

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <62ae486d-621c-4b72-b9fa-d582f80cccd4@redhat.com>
Date: Tue, 11 Mar 2025 13:09:57 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: chia-yu.chang@...ia-bell-labs.com, netdev@...r.kernel.org,
 dsahern@...il.com, davem@...emloft.net, edumazet@...gle.com,
 dsahern@...nel.org, joel.granados@...nel.org, kuba@...nel.org,
 andrew+netdev@...n.ch, horms@...nel.org, pablo@...filter.org,
 kadlec@...filter.org, netfilter-devel@...r.kernel.org,
 coreteam@...filter.org, kory.maincent@...tlin.com, bpf@...r.kernel.org,
 kuniyu@...zon.com, andrew@...n.ch, ij@...nel.org, ncardwell@...gle.com,
 koen.de_schepper@...ia-bell-labs.com, g.white@...leLabs.com,
 ingemar.s.johansson@...csson.com, mirja.kuehlewind@...csson.com,
 cheshire@...le.com, rs.ietf@....at, Jason_Livingood@...cast.com,
 vidhi_goel@...le.com
Subject: Re: [PATCH v7 net-next 09/12] gro: prevent ACE field corruption &
 better AccECN handling

On 3/5/25 11:38 PM, chia-yu.chang@...ia-bell-labs.com wrote:
> From: Ilpo Järvinen <ij@...nel.org>
> 
> There are important differences in how the CWR field behaves
> in RFC3168 and AccECN. With AccECN, CWR flag is part of the
> ACE counter and its changes are important so adjust the flags
> changed mask accordingly.
> 
> Also, if CWR is there, set the Accurate ECN GSO flag to avoid
> corrupting CWR flag somewhere.
> 
> Signed-off-by: Ilpo Järvinen <ij@...nel.org>
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> ---
>  net/ipv4/tcp_offload.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index a4cea85288ff..ef12aee5deb4 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -329,7 +329,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb,
>  	th2 = tcp_hdr(p);
>  	flush = (__force int)(flags & TCP_FLAG_CWR);
>  	flush |= (__force int)((flags ^ tcp_flag_word(th2)) &
> -		  ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH));
> +		  ~(TCP_FLAG_FIN | TCP_FLAG_PSH));
>  	flush |= (__force int)(th->ack_seq ^ th2->ack_seq);
>  	for (i = sizeof(*th); i < thlen; i += 4)
>  		flush |= *(u32 *)((u8 *)th + i) ^
> @@ -405,7 +405,7 @@ void tcp_gro_complete(struct sk_buff *skb)
>  	shinfo->gso_segs = NAPI_GRO_CB(skb)->count;
>  
>  	if (th->cwr)
> -		shinfo->gso_type |= SKB_GSO_TCP_ECN;
> +		shinfo->gso_type |= SKB_GSO_TCP_ACCECN;
>  }
>  EXPORT_SYMBOL(tcp_gro_complete);

To recap: when an host implementing the above will receive a GSO_TCP_ECN
train transmitted by a RFC3168 endpoint, it will re-assemble it in 2
packets: a GSO one with !th->cwr and a non GSO one with th->cwr set.

When receiving a GSO train with constant CWR set on all the wire
packets, it will assemble it in a single SKB_GSO_TCP_ACCECN packet.

I think should work correctly.

Side note: the SKB_GSO_TCP_ACCECN flag is required: NETIF_F_TSO_ECN
enabled driver will likely unconditionally apply RFC3168-like TSO to any
GSO packet carrying the CWR flag, regardless of the skb gso_type.

@Eric: are you ok with this change?

Thanks,

Paolo