| From fd1074159b670f74a8afa347a9b343ab4b540d5c Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 25 Oct 2021 10:59:03 +1100 |
| Subject: tcp: don't free a FIN sk_buff in tcp_remove_empty_skb() |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Jon Maxwell <jmaxwell37@gmail.com> |
| |
| [ Upstream commit cf12e6f9124629b18a6182deefc0315f0a73a199 ] |
| |
| v1: Implement a more general statement as recommended by Eric Dumazet. The |
| sequence number will be advanced, so this check will fix the FIN case and |
| other cases. |
| |
| A customer reported sockets stuck in the CLOSING state. A Vmcore revealed that |
| the write_queue was not empty as determined by tcp_write_queue_empty() but the |
| sk_buff containing the FIN flag had been freed and the socket was zombied in |
| that state. Corresponding pcaps show no FIN from the Linux kernel on the wire. |
| |
| Some instrumentation was added to the kernel and it was found that there is a |
| timing window where tcp_sendmsg() can run after tcp_send_fin(). |
| |
| tcp_sendmsg() will hit an error, for example: |
| |
| 1269 ▹ if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))↩ |
| 1270 ▹ ▹ goto do_error;↩ |
| |
| tcp_remove_empty_skb() will then free the FIN sk_buff as "skb->len == 0". The |
| TCP socket is now wedged in the FIN-WAIT-1 state because the FIN is never sent. |
| |
| If the other side sends a FIN packet the socket will transition to CLOSING and |
| remain that way until the system is rebooted. |
| |
| Fix this by checking for the FIN flag in the sk_buff and don't free it if that |
| is the case. Testing confirmed that fixed the issue. |
| |
| Fixes: fdfc5c8594c2 ("tcp: remove empty skb from write queue in error cases") |
| Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com> |
| Reported-by: Monir Zouaoui <Monir.Zouaoui@mail.schwarz> |
| Reported-by: Simon Stier <simon.stier@mail.schwarz> |
| Reviewed-by: Eric Dumazet <edumazet@google.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/ipv4/tcp.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c |
| index 5c8d0fb498256..9f53d25e047e3 100644 |
| --- a/net/ipv4/tcp.c |
| +++ b/net/ipv4/tcp.c |
| @@ -955,7 +955,7 @@ static int tcp_send_mss(struct sock *sk, int *size_goal, int flags) |
| */ |
| static void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb) |
| { |
| - if (skb && !skb->len) { |
| + if (skb && TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) { |
| tcp_unlink_write_queue(skb, sk); |
| if (tcp_write_queue_empty(sk)) |
| tcp_chrono_stop(sk, TCP_CHRONO_BUSY); |
| -- |
| 2.33.0 |
| |