[PATCH net] tcp: free saved_syn after an RCU grace period

[Yingjie Wang]

tcp_reqsk_record_syn() allocates saved_syn separately from its request
socket.  bpf_sock_ops_get_syn() can load that pointer while processing a
retransmitted SYN under RCU.

There are two ownership transfers which can let another CPU reclaim the
allocation while such a reader is still using it.

First, tcp_move_syn() transfers req->saved_syn to a newly created child
socket.  The child can be closed and tcp_saved_syn_free() can reclaim the
allocation before a reader of the old request finishes.

Second, inet_reqsk_clone() shallow-copies request_sock during request
migration.  The migrated clone becomes the owner after
reqsk_migrate_reset() clears the old request's pointer, but a reader which
already loaded the pointer from the old request can still be using it when
the clone is released.

Both request and full TCP sockets currently free saved_syn with plain
kfree().  Embed an rcu_head in struct saved_syn and use kfree_rcu() for
both teardown paths.  This follows commit e3f33b0a1d89 ("ipv4: free
inet_opt and ireq_opt after an RCU grace period"), which fixed the same
ownership-transfer pattern for IPv4 options.

The issue was confirmed with KASAN on v7.3-16007-g36aad21d50d1 using
temporary kernel instrumentation to widen the reader and request-migration
window.  KASAN reported a slab-use-after-free read in
bpf_sock_ops_get_syn(), with the allocation freed by __reqsk_free() from
reqsk_timer_handler().  The same explicit migration test completed without
a report with this change.  A patched kernel without the delay
instrumentation also completed a successful migration and seven saved-SYN
reads without a KASAN report.

Full allmodconfig and allyesconfig builds completed with W=1.  Existing
warnings outside the modified files were made non-fatal with
KCFLAGS=-Wno-error; the modified files emitted none.

Fixes: 0813a841566f ("bpf: tcp: Allow bpf prog to write and parse TCP header option")
Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yingjie Wang <x-professor@xxxxxxxxxxx>
---
 include/linux/tcp.h             | 2 +-
 include/net/request_sock.h      | 2 ++
 net/ipv4/inet_connection_sock.c | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d18f7bc1e5d0..dd3b097daac9 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -624,7 +624,7 @@ static inline void tcp_move_syn(struct tcp_sock *tp,
 
 static inline void tcp_saved_syn_free(struct tcp_sock *tp)
 {
-	kfree(tp->saved_syn);
+	kfree_rcu(tp->saved_syn, rcu);
 	tp->saved_syn = NULL;
 }
 
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 5a9c826a7092..b6ccfc31147d 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -16,6 +16,7 @@
 #include <linux/types.h>
 #include <linux/bug.h>
 #include <linux/refcount.h>
+#include <linux/rcupdate.h>
 
 #include <net/sock.h>
 #include <net/rstreason.h>
@@ -39,6 +40,7 @@ struct request_sock_ops {
 };
 
 struct saved_syn {
+	struct rcu_head rcu;
 	u32 mac_hdrlen;
 	u32 network_hdrlen;
 	u32 tcp_hdrlen;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 93b306d91b6d..a2622eccaff1 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -908,7 +908,7 @@ void __reqsk_free(struct request_sock *req)
 	req->rsk_ops->destructor(req);
 	if (req->rsk_listener)
 		sock_put(req->rsk_listener);
-	kfree(req->saved_syn);
+	kfree_rcu(req->saved_syn, rcu);
 	kmem_cache_free(req->rsk_ops->slab, req);
 }
 

base-commit: 902407bc588575af0ceae5de4f635fe1e024f258
-- 
2.43.0