linux-kernel - [PATCH 3.16 126/245] tcp: do not send empty skb from tcp_write

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: <lsq.1587683028.868791916@decadent.org.uk>
Date:   Fri, 24 Apr 2020 00:05:53 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Christoph Paasch" <cpaasch@...le.com>,
        "Soheil Hassas Yeganeh" <soheil@...gle.com>,
        "Neal Cardwell" <ncardwell@...gle.com>,
        "Eric Dumazet" <edumazet@...gle.com>,
        "Jakub Kicinski" <jakub.kicinski@...ronome.com>,
        "Jason Baron" <jbaron@...mai.com>
Subject: [PATCH 3.16 126/245] tcp: do not send empty skb from tcp_write_xmit()

3.16.83-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Dumazet <edumazet@...gle.com>

commit 1f85e6267caca44b30c54711652b0726fadbb131 upstream.

Backport of commit fdfc5c8594c2 ("tcp: remove empty skb from
write queue in error cases") in linux-4.14 stable triggered
various bugs. One of them has been fixed in commit ba2ddb43f270
("tcp: Don't dequeue SYN/FIN-segments from write-queue"), but
we still have crashes in some occasions.

Root-cause is that when tcp_sendmsg() has allocated a fresh
skb and could not append a fragment before being blocked
in sk_stream_wait_memory(), tcp_write_xmit() might be called
and decide to send this fresh and empty skb.

Sending an empty packet is not only silly, it might have caused
many issues we had in the past with tp->packets_out being
out of sync.

Fixes: c65f7f00c587 ("[TCP]: Simplify SKB data portion allocation with NETIF_F_SG.")
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Cc: Christoph Paasch <cpaasch@...le.com>
Acked-by: Neal Cardwell <ncardwell@...gle.com>
Cc: Jason Baron <jbaron@...mai.com>
Acked-by: Soheil Hassas Yeganeh <soheil@...gle.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 net/ipv4/tcp_output.c | 8 ++++++++
 1 file changed, 8 insertions(+)

--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1992,6 +1992,14 @@ static bool tcp_write_xmit(struct sock *
 		    unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
 			break;
 
+		/* Argh, we hit an empty skb(), presumably a thread
+		 * is sleeping in sendmsg()/sk_stream_wait_memory().
+		 * We do not want to send a pure-ack packet and have
+		 * a strange looking rtx queue with empty packet(s).
+		 */
+		if (TCP_SKB_CB(skb)->end_seq == TCP_SKB_CB(skb)->seq)
+			break;
+
 		TCP_SKB_CB(skb)->when = tcp_time_stamp;
 
 		if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp)))