[PATCH net] net/smc: hash socket only after full initialisation in smc_sk_init()

Mahanta Jambigi <[email protected]>
Newsgroups org.kernel.vger.linux-s390,org.kernel.vger.netdev
Message-ID <[email protected]>
smc_sk_init() calls sk->sk_prot->hash(sk) before several fields are
fully initialised: clcsock_release_lock, the saved clcsk_* callbacks,
use_fallback/fallback_rsn, and conn.close_work.  Once hash() returns the
socket is visible to concurrent hash walkers, which can then observe
uninitialised state.

Move hash(sk) to the end of smc_sk_init() so the socket is published
only after it is fully constructed.

Fixes: d0e35656d834 ("net/smc: refactoring initialization of smc sock")
Reviewed-by: Hidayath Khan <[email protected]>
Reviewed-by: Sidraya Jayagond <[email protected]>
Signed-off-by: Mahanta Jambigi <[email protected]>
---
Found by syzbot while testing a diag dump patch that drops the hash
read lock before calling mutex_lock(&smc->clcsock_release_lock).  That
change made the uninitialised mutex reachable for the first time and
triggered DEBUG_LOCKS_WARN_ON(lock->magic != lock) immediately.

syzbot report:
https://ci.syzbot.org/series/ab0d12fd-741a-4a0f-9f49-e52b84e94f4a

 net/smc/af_smc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 00403175b740..5e7560417ec7 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -409,13 +409,13 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
 				      "sk_lock-AF_SMC", &smc_key);
 	spin_lock_init(&smc->accept_q_lock);
 	spin_lock_init(&smc->conn.send_lock);
-	sk->sk_prot->hash(sk);
 	mutex_init(&smc->clcsock_release_lock);
 	smc_init_saved_callbacks(smc);
 	smc->limit_smc_hs = net->smc.limit_smc_hs;
 	smc->use_fallback = false; /* assume rdma capability first */
 	smc->fallback_rsn = 0;
 	smc_close_init(smc);
+	sk->sk_prot->hash(sk);
 }
 
 static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
-- 
2.50.1
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.