Re: [syzbot] [net?] general protection fault in kernel_sock_shutdown (4)

[Arjan van de Ven]

Unfortunately the AI had a burp and did not write out the proper URL
for analysis data; it should have been

http://oops.fenrus.org/reports/lkml/69ea344f.a00a0220.17a17.0040.GAE_google.com/report.html

and in addition, it made a candidate patch (below)











From: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
Subject: [PATCH] RDMA/rxe: fix double-release race on UDP tunnel socket teardown

This patch is based on a BUG as reported at
https://lore.kernel.org/r/69ea344f.a00a0220.17a17.0040.GAE@xxxxxxxxxx.

The Soft RoCE (RXE) driver stores per-network-namespace UDP tunnel
sockets for IPv4 and IPv6 encapsulation. Two independent code paths
tear these sockets down: rxe_ns_exit(), called when a network
namespace is destroyed, and rxe_net_del(), called when an RDMA link
is deleted via netlink. Both paths read the per-namespace socket
pointer and call udp_tunnel_sock_release() on it.

A time-of-check/time-of-use (TOCTOU) race exists in rxe_net_del().
It reads the socket pointer via rxe_ns_pernet_sk4(), then passes it
to rxe_sock_put() for release. If rxe_ns_exit() runs concurrently
between the read and the release, it clears the pointer and calls
udp_tunnel_sock_release() first, causing sock_release() to set
sock->ops = NULL. When rxe_net_del() then calls
udp_tunnel_sock_release() on the same socket, kernel_sock_shutdown()
dereferences the now-NULL sock->ops, triggering a KASAN null-ptr-deref
at offset 0x68 (the shutdown function pointer in struct proto_ops).

A minimal alternative would guard against NULL sock->ops inside
udp_tunnel_sock_release() before calling kernel_sock_shutdown(). That
treats the symptom rather than the root cause and leaves the
double-release of socket state intact.

Add rxe_ns_pernet_take_sk4() and rxe_ns_pernet_take_sk6() which use
xchg() to atomically swap the per-namespace socket pointer to NULL
and return the old value. Replace the non-atomic reads in
rxe_net_del() with these take variants, and release the socket
directly via udp_tunnel_sock_release() without going through
rxe_sock_put().

Whichever teardown path executes take first claims ownership of the
socket; the second caller gets NULL and skips the release, closing
the double-release window.

Link: https://lore.kernel.org/r/69ea344f.a00a0220.17a17.0040.GAE@xxxxxxxxxx
Oops-Analysis: http://oops.fenrus.org/reports/lkml/69ea344f.a00a0220.17a17.0040.GAE_google.com/report.html
Fixes: 13f2a53c2a71 ("RDMA/rxe: Add net namespace support for IPv4/IPv6 sockets")
Fixes: f1327abd6abe ("RDMA/rxe: Support RDMA link creation and destruction per net namespace")
Assisted-by: GitHub Copilot patcher:claude linux-kernel-oops-x86.
Signed-off-by: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
Cc: linux-rdma@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: Zhu Yanjun <zyjzyj2000@xxxxxxxxx>
Cc: Jason Gunthorpe <jgg@xxxxxxxx>
Cc: Leon Romanovsky <leon@xxxxxxxxxx>

---
 drivers/infiniband/sw/rxe/rxe_net.c |    8 ++++----
 drivers/infiniband/sw/rxe/rxe_ns.c  |   14 ++++++++++++++
 drivers/infiniband/sw/rxe/rxe_ns.h  |    7 +++++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 50a2cb5405e22..4f604636cb7b4 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -655,13 +655,13 @@ void rxe_net_del(struct ib_device *dev)
 
 	net = dev_net(ndev);
 
-	sk = rxe_ns_pernet_sk4(net);
+	sk = rxe_ns_pernet_take_sk4(net);
 	if (sk)
-		rxe_sock_put(sk, rxe_ns_pernet_set_sk4, net);
+		udp_tunnel_sock_release(sk->sk_socket);
 
-	sk = rxe_ns_pernet_sk6(net);
+	sk = rxe_ns_pernet_take_sk6(net);
 	if (sk)
-		rxe_sock_put(sk, rxe_ns_pernet_set_sk6, net);
+		udp_tunnel_sock_release(sk->sk_socket);
 
 	dev_put(ndev);
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_ns.c b/drivers/infiniband/sw/rxe/rxe_ns.c
index 8b9d734229b24..d9d376e3c670f 100644
--- a/drivers/infiniband/sw/rxe/rxe_ns.c
+++ b/drivers/infiniband/sw/rxe/rxe_ns.c
@@ -91,6 +91,13 @@ void rxe_ns_pernet_set_sk4(struct net *net, struct sock *sk)
 	synchronize_rcu();
 }
 
+struct sock *rxe_ns_pernet_take_sk4(struct net *net)
+{
+	struct rxe_ns_sock *ns_sk = net_generic(net, rxe_pernet_id);
+
+	return xchg((__force struct sock **)&ns_sk->rxe_sk4, NULL);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 struct sock *rxe_ns_pernet_sk6(struct net *net)
 {
@@ -111,6 +118,13 @@ void rxe_ns_pernet_set_sk6(struct net *net, struct sock *sk)
 	rcu_assign_pointer(ns_sk->rxe_sk6, sk);
 	synchronize_rcu();
 }
+
+struct sock *rxe_ns_pernet_take_sk6(struct net *net)
+{
+	struct rxe_ns_sock *ns_sk = net_generic(net, rxe_pernet_id);
+
+	return xchg((__force struct sock **)&ns_sk->rxe_sk6, NULL);
+}
 #endif /* IPV6 */
 
 int rxe_namespace_init(void)
diff --git a/drivers/infiniband/sw/rxe/rxe_ns.h b/drivers/infiniband/sw/rxe/rxe_ns.h
index 4da2709e6b714..9d9a5106b77c8 100644
--- a/drivers/infiniband/sw/rxe/rxe_ns.h
+++ b/drivers/infiniband/sw/rxe/rxe_ns.h
@@ -5,10 +5,17 @@
 
 struct sock *rxe_ns_pernet_sk4(struct net *net);
 void rxe_ns_pernet_set_sk4(struct net *net, struct sock *sk);
+struct sock *rxe_ns_pernet_take_sk4(struct net *net);
 
 #if IS_ENABLED(CONFIG_IPV6)
 void rxe_ns_pernet_set_sk6(struct net *net, struct sock *sk);
 struct sock *rxe_ns_pernet_sk6(struct net *net);
+struct sock *rxe_ns_pernet_take_sk6(struct net *net);
 #else /* IPv6 */
 static inline struct sock *rxe_ns_pernet_sk6(struct net *net)
 {
@@ -18,6 +25,10 @@ static inline void rxe_ns_pernet_set_sk6(struct net *net, struct sock *sk)
 {
 }
 
+static inline struct sock *rxe_ns_pernet_take_sk6(struct net *net)
+{
+	return NULL;
+}
 #endif /* IPv6 */
 
 int rxe_namespace_init(void);