[PATCH net v2 5/8] tcp: do not inherit out_of_order_queue from parent

A child gets a copy of the parent's out_of_order_queue, which can be non
empty when/if parent morphs from listener to active session. Parent and
child then point at the same rbtree.

The parent is no longer a listener, so inet_csk_reqsk_queue_add() forgets
the child immediately, and tcp_disconnect() frees the skbs the parent
still owns. The parent's own root and ooo_last_skb are left alone, so it
keeps using those skbs. That is a use-after-free, and the parent frees
them a second time when it closes.

In short:

  peers                               the socket owner

                                      socket(AF_INET6)
                                      setsockopt(TCP_DEFER_ACCEPT, 30)
                                      bind(), listen()
  connect(the listener)
      // bare ACK deferred, the request stays
                                      connect(AF_UNSPEC)
                                      connect(the new peer)
  send(the new connection)
      // it starts one byte past rcv_nxt, so it lands out of order
      tcp_data_queue()
        tcp_data_queue_ofo()  // the parent's queue fills up
  send(the old connection)
      tcp_check_req()
        tcp_v6_syn_recv_sock()
          tcp_create_openreq_child()  // the child gets the same rbtree
        inet_csk_complete_hashdance()
          inet_csk_reqsk_queue_add()
            inet_child_forget()
              tcp_disconnect()
                skb_rbtree_purge()  // the parent's skbs are freed
  send(the new connection)
      tcp_data_queue()
        tcp_data_queue_ofo()
          rb_link_node()  // use-after-free write

KASAN log:

  BUG: KASAN: slab-use-after-free in tcp_data_queue+0x1708/0x1df0
  Write of size 8 at addr ffff888012ad5408 by task repro/135
  ...
  Call Trace:
   <IRQ>
   tcp_data_queue+0x1708/0x1df0
   tcp_rcv_established+0x441/0x1830
   tcp_v4_do_rcv+0x47e/0x730
   tcp_v4_rcv+0x171c/0x2040
   ip_protocol_deliver_rcu+0x5c/0x280
   ip_local_deliver_finish+0x15a/0x2e0
   ip_local_deliver+0x107/0x370
   ip_rcv+0x3b1/0x3d0
  ...
  Allocated by task 135:
   __alloc_skb+0xd0/0x370
   alloc_skb_with_frags+0x7d/0x330
   sock_alloc_send_pskb+0x490/0x4e0
   raw_sendmsg+0xd52/0x1850
  ...
  Freed by task 133:
   kmem_cache_free+0x26f/0x5f0
   skb_rbtree_purge+0x73/0x90
   tcp_disconnect+0x1be/0xd60
   inet_child_forget+0x41/0x140
   inet_csk_complete_hashdance+0x4d0/0x520
   tcp_check_req+0x9f9/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
  ...
  The buggy address belongs to the object at ffff888012ad5400
   which belongs to the cache skbuff_head_cache of size 232
  The buggy address is located 8 bytes inside of
   freed 232-byte region [ffff888012ad5400, ffff888012ad54e8)

We need to make sure this can not happen, by initializing the queue after
socket cloning.

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

Fixes: 9f5afeae5152 ("tcp: use an RB tree for ooo receive queue")
Cc: [email protected]
Signed-off-by: Hyunwoo Kim <[email protected]>
---
Changes in v2:
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/[email protected]/
---
 net/ipv4/tcp_minisocks.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 0c3b35a381e327..7fe318d9e0aed2 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -591,6 +591,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 	newtp->total_retrans = req->num_retrans;
 
 	tcp_init_xmit_timers(newsk);
+	newtp->out_of_order_queue = RB_ROOT;
 	WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);
 
 	if (sock_flag(newsk, SOCK_KEEPOPEN))
-- 
2.43.0