[PATCH net v2 4/8] net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM

sk_clone() allocates the child from sk->sk_prot, and IPV6_ADDRFORM can
change sk_prot under it. The conversion requires the socket to be
established, and a listener gets there with connect(AF_UNSPEC) followed
by connect().

tcp_check_req() completes a request without the listener lock, so it can
run while the conversion is in progress. IPV6_ADDRFORM stores sk_prot
before icsk_af_ops, so tcp_check_req() can still call
tcp_v6_syn_recv_sock() once sk_prot is tcp_prot. The child then comes
from tcp_prot's slab while the AF_INET6 code treats it as a tcp6_sock.

tcp_inet6_sk() is a fixed offset into tcp6_sock, and in a child sized by
tcp_prot that offset is the end of the object. The ipv6_pinfo copy is
therefore a slab out-of-bounds write of sizeof(struct ipv6_pinfo) bytes
past the child.

The out-of-bounds address is also stored in the child's pinet6, so
everything that reaches the socket through inet6_sk() keeps writing
there. A request that arrived over IPv4 takes the same copy in
tcp_v6_mapped_child_init().

In short:

  client                              the socket owner

                                      socket(AF_INET6)
                                      setsockopt(TCP_DEFER_ACCEPT, 30)
                                      bind(), listen()
  connect(::1)
      // bare ACK deferred, request stays
  send()
      // tcp_check_req() ->
      //   picks tcp_v6_syn_recv_sock()
                                      connect(AF_UNSPEC)
                                      connect(::ffff:127.0.0.1)
                                      setsockopt(IPV6_ADDRFORM, PF_INET)
                                          // sk_prot = tcp_prot
      // sk_clone() -> child from the TCP slab
      // tcp_v6_syn_recv_sock() -> memcpy(ipv6_pinfo)
      // out-of-bounds write of 128 bytes

KASAN log:

  BUG: KASAN: slab-out-of-bounds in tcp_v6_syn_recv_sock+0x297/0xce0
  Write of size 128 at addr ffff888015b5f480 by task repro/161
  ...
  Call Trace:
   <IRQ>
   __asan_memcpy+0x3c/0x60
   tcp_v6_syn_recv_sock+0x297/0xce0
   tcp_check_req+0x374/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
   ip6_protocol_deliver_rcu+0x1aa/0x870
   ip6_input_finish+0xac/0x1a0
   ip6_input+0xe5/0x490
   ipv6_rcv+0x2a0/0x3d0
  ...
  Allocated by task 161:
   sk_prot_alloc+0x45/0x170
   sk_clone+0x49/0x960
   inet_csk_clone_lock+0x29/0x2c0
   tcp_create_openreq_child+0x2a/0xf20
   tcp_v6_syn_recv_sock+0x14e/0xce0
   tcp_check_req+0x374/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
  ...
  The buggy address belongs to the object at ffff888015b5e800
   which belongs to the cache TCP of size 3200
  The buggy address is located 0 bytes to the right of
   allocated 3200-byte region [ffff888015b5e800, ffff888015b5f480)

Checking sk_prot before the clone does not help. It can change between
that check and the read inside sk_clone(). Use sk_prot_creator instead.
It is set once in sk_alloc() and never changes, and the socket is
already freed back through it. No caller that replaces sk_prot installs
a proto with a larger obj_size than the creator, so the child gets the
size the parent object actually has.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
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/core/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 1ad41904db25b4..098e58b40f304b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2479,7 +2479,7 @@ static void sk_init_common(struct sock *sk)
 struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 		      bool lock)
 {
-	struct proto *prot = READ_ONCE(sk->sk_prot);
+	struct proto *prot = sk->sk_prot_creator;
 	struct sk_filter *filter;
 	bool is_charged = true;
 	struct sock *newsk;
-- 
2.43.0