[PATCH net v2 8/8] tcp: do not inherit retransmit state from parent

[Hyunwoo Kim <imv4bel@gmail.com>]

A child gets a copy of the parent's retransmit state when it is cloned.
On a listener it is all zero, which is why commit eb2c80ca87b1 ("tcp: do
not clear packets_out in tcp_create_openreq_child()") and
commit 5c701549c9a6 ("tcp: move retrans_out, sacked_out, tlp_high_seq,
last_oow_ack_time init to tcp_disconnect()") dropped the initialization
of packets_out, retrans_out and sacked_out here. lost_out,
retransmit_skb_hint and highest_sack have never been cleared here.

The parent can morph from listener to active session while a request is
still being processed, and connect(AF_UNSPEC) followed by connect()
gets it there. tcp_check_req() does not hold the listener lock, so
nothing pins the parent's state between the TCP_LISTEN test and the
clone. The child then copies the counters and the two pointers into the
parent's retransmit queue.

sk_clone() sets sk_send_head to NULL, and tcp_rtx_queue is unioned with
it, so the child's retransmit queue is empty. That does not help.
packets_out is set again as soon as the child sends anything, and
tcp_xmit_retransmit_queue() picks the copied hint over the queue head.
When the parent disconnects, tcp_write_queue_purge() frees those skbs,
but the pointers the child copied are left alone. The child then gets
an ACK, enters the retransmit path and writes into a freed skb.

In short:

  socket(AF_INET) -> setsockopt(TCP_DEFER_ACCEPT, 30) -> bind -> listen
      // a client connects and sends one byte. the kernel processes
      // that segment while the steps below run
  connect(AF_UNSPEC)              // stop listening
  connect(peer)                   // become an active session
  send() repeatedly               // lower IP_TTL so the peer's IP_MINTTL
                                  // drops most of them, and let one
                                  // through to get a SACK
      // parent: packets_out 6, sacked_out 1, lost_out 5, retrans_out 1
      // retransmit_skb_hint points at an skb in the parent's queue
      // the leftover request completes and copies this state
  connect(AF_UNSPEC)              // those skbs are freed
  listen()                        // or the child is dropped
  accept()
      // the child sends data, one segment is lost, and the ACK that
      // comes back takes the retransmit path to the copied hint

KASAN log:

  BUG: KASAN: slab-use-after-free in __pskb_trim_head+0x66b/0x900
  Write of size 16 at addr ffff888008141530 by task repro/76
  ...
  Call Trace:
   __pskb_trim_head+0x66b/0x900
   tcp_trim_head+0x69/0x540
   __tcp_retransmit_skb+0x14e/0x26b0
   tcp_retransmit_skb+0x1b/0x250
   tcp_xmit_retransmit_queue.part.0+0x3b1/0x970
   tcp_ack+0x3382/0x7430
   tcp_rcv_established+0x631/0x3a00
   tcp_v4_do_rcv+0x449/0x960
   __release_sock+0x1f2/0x2a0
   release_sock+0x176/0x1d0
   tcp_sendmsg+0x30/0x40
   __sys_sendto+0x316/0x380
   __x64_sys_sendto+0xdb/0x1b0
  ...
  Allocated by task 76:
   __alloc_skb+0x11e/0x890
   tcp_stream_alloc_skb+0x2c/0x5c0
   tcp_sendmsg_locked+0x1377/0x3df0
   tcp_sendmsg+0x26/0x40
   __sys_sendto+0x316/0x380
  ...
  Freed by task 80:
   skb_release_data+0x554/0x810
   __kfree_skb+0x42/0x60
   tcp_write_queue_purge+0x6ef/0xf40
   tcp_disconnect+0x2fc/0x1e10
   __inet_stream_connect+0x6d0/0xdf0
   inet_stream_connect+0x52/0xa0
   __sys_connect+0xfc/0x130
  ...
  The buggy address belongs to the object at ffff888008141380
   which belongs to the cache skbuff_small_head of size 704

We need to make sure this can not happen, by clearing them after socket
cloning. A listener always has them zero, so an ordinary passive open is
not affected. Clearing only the pointers is not enough: the counters
would then describe a retransmit queue the child does not have, and
tcp_fastretrans_alert() and tcp_retransmit_timer() warn.

Very similar to commit 8b485ce69876 ("tcp: do not inherit fastopen_req
from parent")

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/tcp_minisocks.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 7fe318d9e0aed2..2fde196dd302f5 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -660,6 +660,12 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 	tcp_ecn_openreq_child(newsk, req, skb);
 	newtp->fastopen_req = NULL;
 	RCU_INIT_POINTER(newtp->fastopen_rsk, NULL);
+	newtp->packets_out = 0;
+	newtp->retrans_out = 0;
+	newtp->sacked_out = 0;
+	newtp->lost_out = 0;
+	newtp->retransmit_skb_hint = NULL;
+	newtp->highest_sack = NULL;
 
 	newtp->bpf_chg_cc_inprogress = 0;
 	tcp_bpf_clone(sk, newsk);
-- 
2.43.0