Message-ID: <CANn89iLYHdtAFSjSW+cSN0Td_V3B+V05hHnGeop5Y+hjWEt_HA@mail.gmail.com>
Date: Fri, 22 Aug 2025 01:53:38 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: MingMing Wang <mii.w@...ux.alibaba.com>
Cc: ncardwell@...gle.com, kuniyu@...gle.com, davem@...emloft.net, 
	dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	ycheng@...gle.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Dust Li <dust.li@...ux.alibaba.com>
Subject: Re: [RFC net] tcp: Fix orphaned socket stalling indefinitely in FIN-WAIT-1

On Thu, Aug 21, 2025 at 11:04 PM MingMing Wang <mii.w@...ux.alibaba.com> wrote:
>
> From: MingMing Wang <mii.w@...ux.alibaba.com>
>
> An orphaned TCP socket can stall indefinitely in FIN-WAIT-1
> if the following conditions are met:
> 1. net.ipv4.tcp_retries2 is set to a value ≤ 8;
> 2. The peer advertises a zero window, and the window never reopens.
>
> Steps to reproduce:
> 1. Set up two instances with nmap installed: one will act as the server
>    the other as the client
> 2. Execute on the server:
>    a. lower rmem : `sysctl -w net.ipv4.tcp_rmem="16 32 32"`
>    b. start a listener: `nc -l -p 1234`
> 3. Execute on the client:
>    a. lower tcp_retries2: `sysctl -w net.ipv4.tcp_retries2=8`
>    b. send pakcets: `cat /dev/zero | nc <server-ip> 1234`
>    c. after five seconds, stop the process: `killall nc`
> 4. Execute on the server: `killall -STOP nc`
> 5. Expected abnormal result: using `ss` command, we'll notice that the
>    client connection remains stuck in the FIN_WAIT1 state, and the
>    backoff counter always be 8 and no longer increased, as shown below:
>    ```

Hi MingMing

Please prepare and share with us a packetdrill test, instead of this
'repro', which is the old way of describing things :/

- This will be easier for us to understand the issue.

- It will be added to existing tests in tools/testing/selftests/net/packetdrill
if your patch is accepted, so that we can make sure future changes are
not breaking this again.

Ideally, you should attach this packetdrill test in a second patch
(thus sending a series of two patches)

Thank you.

>    FIN-WAIT-1 0      1389    172.16.0.2:50316    172.16.0.1:1234
>          cubic wscale:2,7 rto:201 backoff:8 rtt:0.078/0.007 mss:36
>                  ... other fields omitted ...
>    ```
> 6. If we set tcp_retries2 to 15 and repeat the steps above, the FIN_WAIT1
>    state will be forcefully reclaimed after about 5 minutes.
>
> During the zero-window probe retry process, it will check whether the
> current connection is alive or not. If the connection is not alive and
> the counter of retries exceeds the maximum allowed `max_probes`, retry
> process will be terminated.
>
> In our case, when we set `net.ipv4.tcp_retries2` to 8 or a less value,
> according to the current implementation, the `icsk->icsk_backoff` counter
> will be capped at `net.ipv4.tcp_retries2`. The value calculated by
> `inet_csk_rto_backoff` will always be too small, which means the
> computed backoff duration will always be less than rto_max. As a result,
> the alive check will always return true. The condition before the
> `goto abort` statement is an logical AND condition, the abort branch
> can never be reached.
>
> So, the TCP retransmission backoff mechanism has two issues:
>
> 1. `icsk->icsk_backoff` should monotonically increase during probe
>    transmission and, upon reaching the maximum backoff limit, the
>    connection should be terminated. However, the backoff value itself
>    must not be capped prematurely — it should only control when to abort.
>
> 2. The condition for orphaned connection abort was incorrectly based on
>    connection liveness and probe count. It should instead consider whether
>    the number of orphaned probes exceeds the intended limit.
>
> To fix this, introduce a local variable `orphan_probes` to track orphan
> probe attempts separately from `max_probes`, which is used for RTO
> retransmissions. This decouples the two counters and prevents accidental
> overwrites, ensuring correct timeout behavior for orphaned connections.
>
> Fixes: b248230c34970 ("tcp: abort orphan sockets stalling on zero window probes")
> Co-developed-by: Dust Li <dust.li@...ux.alibaba.com>
> Signed-off-by: Dust Li <dust.li@...ux.alibaba.com>
> Co-developed-by: MingMing Wang <mii.w@...ux.alibaba.com>
> Signed-off-by: MingMing Wang <mii.w@...ux.alibaba.com>
>
> ---
> We couldn't determine the rationale behind the following check in tcp_send_probe0():
> ```
> if (icsk->icsk_backoff < READ_ONCE(net->ipv4.sysctl_tcp_retries2))
>     icsk->icsk_backoff++;
> ```
>
> This condition appears to be the root cause of the observed stall.
> However, it has existed in the kernel for over 20 years — which suggests
> there might be a historical or subtle reason for its presence.
>
> We would greatly appreciate it if anyone could shed
> ---
>  net/ipv4/tcp_output.c | 4 +---
>  net/ipv4/tcp_timer.c  | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index caf11920a878..21795d696e38 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -4385,7 +4385,6 @@ void tcp_send_probe0(struct sock *sk)
>  {
>         struct inet_connection_sock *icsk = inet_csk(sk);
>         struct tcp_sock *tp = tcp_sk(sk);
> -       struct net *net = sock_net(sk);
>         unsigned long timeout;
>         int err;
>
> @@ -4401,8 +4400,7 @@ void tcp_send_probe0(struct sock *sk)
>
>         icsk->icsk_probes_out++;
>         if (err <= 0) {
> -               if (icsk->icsk_backoff < READ_ONCE(net->ipv4.sysctl_tcp_retries2))
> -                       icsk->icsk_backoff++;
> +               icsk->icsk_backoff++;

I think we need to have a cap, otherwise we risk overflows in
inet_csk_rto_backoff()


>                 timeout = tcp_probe0_when(sk, tcp_rto_max(sk));
>         } else {
>                 /* If packet was not sent due to local congestion,
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index a207877270fb..4dba2928e1bf 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -419,9 +419,9 @@ static void tcp_probe_timer(struct sock *sk)
>         if (sock_flag(sk, SOCK_DEAD)) {
>                 unsigned int rto_max = tcp_rto_max(sk);
>                 const bool alive = inet_csk_rto_backoff(icsk, rto_max) < rto_max;
> +               int orphan_probes = tcp_orphan_retries(sk, alive);
>
> -               max_probes = tcp_orphan_retries(sk, alive);
> -               if (!alive && icsk->icsk_backoff >= max_probes)
> +               if (!alive || icsk->icsk_backoff >= orphan_probes)
>                         goto abort;
>                 if (tcp_out_of_resources(sk, true))
>                         return;
> --
> 2.46.0
>

Powered by blists - more mailing lists