[PATCH net v2 3/8] ipv6: fix request socket use-after-free after IPV6_ADDRFORM

Hyunwoo Kim <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kernel.vger.stable
Message-ID <[email protected]>
IPV6_ADDRFORM turns an AF_INET6 TCP socket into an AF_INET one. It requires
the socket to be established, and a listener can get there with
connect(AF_UNSPEC) followed by connect(). Request sockets queued while it
was listening are still there: inet_csk_listen_stop() leaves them in the
ehash, and their timers only drop them while the socket is not listening,
so making it listen again keeps them alive.

A request that arrived over IPv6 was hashed with inet6_ehashfn(). Its child
is cloned from the converted socket and hashed with inet_ehashfn(), so it
belongs in a different bucket.

inet_ehash_insert() locks the child's bucket, warns about the mismatching
hashes, and replaces the request with the child in the request's own bucket
anyway. reqsk_queue_unlink() locks the bucket the request is really in, so
there is no synchronization between the two. Both can see the request still
hashed and both can drop the reference the ehash holds.

The extra put takes the request's refcount to zero too early, so it is
freed while it is still on the listener's accept queue. The listener is
then closed, and inet_csk_listen_stop() reads the freed request and writes
to it in reqsk_put().

In short:

  socket(AF_INET6) -> setsockopt(TCP_DEFER_ACCEPT, 120) -> bind -> listen
      // a native IPv6 client connects and sends nothing
      // the request stays in the bucket inet6_ehashfn() picked
  connect(AF_UNSPEC)                    // stop listening
  connect(a v4-mapped peer)             // become established
  setsockopt(IPV6_ADDRFORM, PF_INET)    // become an AF_INET socket
  connect(AF_UNSPEC), listen()          // listen again
      // the client sends data and the leftover request completes
  close()                               // use-after-free

KASAN log:

  BUG: KASAN: slab-use-after-free in inet_csk_listen_stop+0x1c2/0x760
  Write of size 4 at addr ffff888010b7cbe0 by task init/1
  ...
  Call Trace:
   inet_csk_listen_stop+0x1c2/0x760
   __tcp_close+0x6c1/0x7b0
   tcp_close+0x23/0x90
   inet_release+0x93/0x100
   __sock_release+0x66/0x130
   sock_close+0x18/0x20
   __fput+0x1f0/0x4c0
   fput_close_sync+0xd2/0x170
   __x64_sys_close+0x55/0x90
  ...
  Allocated by task 0:
   inet_reqsk_alloc+0x8c/0x320
   tcp_conn_request+0x324/0x11f0
   tcp_rcv_state_process+0x2ff/0x2cf0
   tcp_v6_do_rcv+0x326/0xc30
   tcp_v6_rcv+0x1e07/0x1e90
  ...
  Freed by task 0:
   slab_free_after_rcu_debug+0xc5/0x200
   rcu_core+0x4dc/0xd20
  ...
  Last potentially related work creation:
   kmem_cache_free+0x11d/0x5f0
   tcp_v6_rcv+0xb30/0x1e90
  ...
  The buggy address belongs to the object at ffff888010b7cb60
   which belongs to the cache request_sock_TCPv6 of size 352

Refuse the conversion if inet_csk_reqsk_queue_len() is not zero. Nothing
clears that counter when a socket stops listening or listens again, so it
still accounts for the requests left in the ehash. A socket that never
listened is not affected.

Reading the counter once is not enough. inet_csk_reqsk_queue_hash_add()
puts the request in the ehash first and bumps the counter second, so a
setsockopt that lands in between misses a request that is already
reachable. No new request is created once the socket stops listening, and
a SYN that found it while it was still listening is handled inside the RCU
read-side critical section the receive path holds. Waiting for one grace
period before reading again therefore leaves no request that is reachable
but not yet counted.

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: [email protected]
Signed-off-by: Hyunwoo Kim <[email protected]>
---
Changes in v2:
- Read inet_csk_reqsk_queue_len() again after synchronize_rcu(). A request
  is put in the ehash before the counter is bumped, so v1 could be raced.
- Move the check below the TCP_ESTABLISHED and v4-mapped checks. In v1 a
  listener returned -EBUSY or -ENOTCONN depending on whether a peer had a
  half-open connection at that moment.
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/[email protected]/
---
 net/ipv6/ipv6_sockglue.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index b4c977434c2e0a..6d2dd9ae46a964 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -587,6 +587,21 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 				break;
 			}
 
+			if (sk->sk_protocol == IPPROTO_TCP) {
+				if (inet_csk_reqsk_queue_len(sk)) {
+					retv = -EBUSY;
+					break;
+				}
+				/* A SYN that found this socket while it was
+				 * still listening may not be counted yet.
+				 */
+				synchronize_rcu();
+				if (inet_csk_reqsk_queue_len(sk)) {
+					retv = -EBUSY;
+					break;
+				}
+			}
+
 			__ipv6_sock_mc_close(sk);
 			__ipv6_sock_ac_close(sk);
 
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.