Re: [PATCH RFC net] net: sockmap: avoid race between sock_map_destroy() and sk_psock_put()

[Cong Wang <xiyou.wangcong@gmail.com>]

On Thu, Sep 05, 2024 at 09:42:57AM +0300, Dmitry Antipov wrote:
> At https://syzkaller.appspot.com/bug?extid=f363afac6b0ace576f45, syzbot
> has triggered the following race condition:

Are you sure it is due to sockmap code?

I see rds_tcp_accept_one() in the stack trace. This is why I highly
suspect that it is due to RDS code instead of sockmap code.

I have the following patch ready for testing, in case you are
interested.

Thanks.

--------------->

commit 4068420e2c82137ab95d387346c0776a36c69e5d
Author: Cong Wang <cong.wang@bytedance.com>
Date:   Sun Sep 1 17:01:49 2024 -0700

    rds: check sock->sk->sk_user_data conflicts
    
    Signed-off-by: Cong Wang <cong.wang@bytedance.com>

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index 351ac1747224..54ee7f6b8f34 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -134,11 +134,12 @@ void rds_tcp_restore_callbacks(struct socket *sock,
  * it is set.  The absence of RDS_CONN_UP bit protects those paths
  * from being called while it isn't set.
  */
-void rds_tcp_reset_callbacks(struct socket *sock,
-			     struct rds_conn_path *cp)
+int rds_tcp_reset_callbacks(struct socket *sock,
+			    struct rds_conn_path *cp)
 {
 	struct rds_tcp_connection *tc = cp->cp_transport_data;
 	struct socket *osock = tc->t_sock;
+	int ret = 0;
 
 	if (!osock)
 		goto newsock;
@@ -181,21 +182,25 @@ void rds_tcp_reset_callbacks(struct socket *sock,
 newsock:
 	rds_send_path_reset(cp);
 	lock_sock(sock->sk);
-	rds_tcp_set_callbacks(sock, cp);
+	ret = rds_tcp_set_callbacks(sock, cp);
 	release_sock(sock->sk);
+	return ret;
 }
 
 /* Add tc to rds_tcp_tc_list and set tc->t_sock. See comments
  * above rds_tcp_reset_callbacks for notes about synchronization
  * with data path
  */
-void rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp)
+int rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp)
 {
 	struct rds_tcp_connection *tc = cp->cp_transport_data;
 
-	rdsdebug("setting sock %p callbacks to tc %p\n", sock, tc);
 	write_lock_bh(&sock->sk->sk_callback_lock);
-
+	if (sock->sk->sk_user_data) {
+		write_unlock_bh(&sock->sk->sk_callback_lock);
+		return -EBUSY;
+	}
+	rdsdebug("setting sock %p callbacks to tc %p\n", sock, tc);
 	/* done under the callback_lock to serialize with write_space */
 	spin_lock(&rds_tcp_tc_list_lock);
 	list_add_tail(&tc->t_list_item, &rds_tcp_tc_list);
@@ -222,6 +227,7 @@ void rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp)
 	sock->sk->sk_state_change = rds_tcp_state_change;
 
 	write_unlock_bh(&sock->sk->sk_callback_lock);
+	return 0;
 }
 
 /* Handle RDS_INFO_TCP_SOCKETS socket option.  It only returns IPv4
diff --git a/net/rds/tcp.h b/net/rds/tcp.h
index 053aa7da87ef..710cc7fa41af 100644
--- a/net/rds/tcp.h
+++ b/net/rds/tcp.h
@@ -50,8 +50,8 @@ struct rds_tcp_statistics {
 
 /* tcp.c */
 bool rds_tcp_tune(struct socket *sock);
-void rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp);
-void rds_tcp_reset_callbacks(struct socket *sock, struct rds_conn_path *cp);
+int rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp);
+int rds_tcp_reset_callbacks(struct socket *sock, struct rds_conn_path *cp);
 void rds_tcp_restore_callbacks(struct socket *sock,
 			       struct rds_tcp_connection *tc);
 u32 rds_tcp_write_seq(struct rds_tcp_connection *tc);
diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index d89bd8d0c354..695456455aee 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -205,11 +205,15 @@ int rds_tcp_accept_one(struct socket *sock)
 		goto rst_nsk;
 	if (rs_tcp->t_sock) {
 		/* Duelling SYN has been handled in rds_tcp_accept_one() */
-		rds_tcp_reset_callbacks(new_sock, cp);
+		ret = rds_tcp_reset_callbacks(new_sock, cp);
+		if (ret)
+			goto rst_nsk;
 		/* rds_connect_path_complete() marks RDS_CONN_UP */
 		rds_connect_path_complete(cp, RDS_CONN_RESETTING);
 	} else {
-		rds_tcp_set_callbacks(new_sock, cp);
+		ret = rds_tcp_set_callbacks(new_sock, cp);
+		if (ret)
+			goto rst_nsk;
 		rds_connect_path_complete(cp, RDS_CONN_CONNECTING);
 	}
 	new_sock = NULL;