netdev - Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state

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: <88c2e98e-8be8-3a18-81e0-9b2ad89cff75@gmail.com>
Date:   Fri, 9 Apr 2021 18:52:13 +0200
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Manoj Basapathi <manojbm@...eaurora.org>, netdev@...r.kernel.org
Cc:     jgarzik@...ox.com, avem@...emloft.net, shemminger@...tta.com,
        linville@...driver.com, mkubecek@...e.cz, kuba@...nel.org,
        bpf@...earbox.net, dsahern@...il.com, kubakici@...pl,
        sharathv@....qualcomm.com, ssaha@....qualcomm.com,
        vidulak@....qualcomm.com, manojbm@....qualcomm.com,
        subashab@...cinc.com, rpavan@....qualcomm.com,
        Sauvik Saha <ssaha@...eaurora.org>
Subject: Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state



On 4/5/21 7:02 PM, Manoj Basapathi wrote:
> Userspace sends tcp connection (sock) destroy on network switch
> i.e switching the default network of the device between multiple
> networks(Cellular/Wifi/Ethernet).
> 
> Kernel though doesn't send reset for the connections in SYN-SENT state
> and these connections continue to remain.
> Even as per RFC 793, there is no hard rule to not send RST on ABORT in
> this state.
> 
> Modify tcp_abort and tcp_disconnect behavior to send RST for connections
> in syn-sent state to avoid lingering connections on network switch.
> 
> Signed-off-by: Manoj Basapathi <manojbm@...eaurora.org>
> Signed-off-by: Sauvik Saha <ssaha@...eaurora.org>
> ---
>  net/ipv4/tcp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e14fd0c50c10..627a472161fb 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2888,7 +2888,7 @@ static inline bool tcp_need_reset(int state)
>  {
>  	return (1 << state) &
>  	       (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> -		TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> +		TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
>  }
>  
>  static void tcp_rtx_queue_purge(struct sock *sk)
> @@ -2954,8 +2954,7 @@ int tcp_disconnect(struct sock *sk, int flags)
>  		 */
>  		tcp_send_active_reset(sk, gfp_any());
>  		sk->sk_err = ECONNRESET;
> -	} else if (old_state == TCP_SYN_SENT)
> -		sk->sk_err = ECONNRESET;
> +	}
>  
>  	tcp_clear_xmit_timers(sk);
>  	__skb_queue_purge(&sk->sk_receive_queue);
> 

This is a completely buggy patch.

This has been sent to many people but _not_ to TCP maintainers ????

I will send a revert.