Fixes deadlock described in this bug: https://syzkaller.appspot.com/bug?extid=e953a8f3071f5c0a28fd. Specific crash report here: https://syzkaller.appspot.com/text?tag=CrashReport&x=14670e07980000. DESCRIPTION OF ISSUE Deadlock: sk_lock-AF_INET --> &smc->clcsock_release_lock --> rtnl_mutex rtnl_mutex->sk_lock-AF_INET rtnetlink_rcv_msg() acquires rtnl_lock() and calls rtnl_newlink(), which eventually calls gtp_newlink() which calls lock_sock() to attempt to acquire sk_lock. sk_lock-AF_INET->&smc->clcsock_release_lock smc_sendmsg() calls lock_sock() to acquire sk_lock, then calls smc_switch_to_fallback() which attempts to acquire mutex_lock(&smc->...). &smc->clcsock_release_lock->rtnl_mutex smc_setsockopt() calls mutex_lock(&smc->...). smc->...->setsockopt() is called, which calls nf_setsockopt() which attempts to acquire rtnl_lock() in some nested call in start_sync_thread() in ip_vs_sync.c. FIX: In smc_switch_to_fallback(), separate the logic into inline function __smc_switch_to_fallback(). In smc_sendmsg(), lock ordering can be modified and the functionality of smc_switch_to_fallback() is encapsulated in the __smc_switch_to_fallback() function. Signed-off-by: Daniel Yang <danielyangkang@xxxxxxxxx> Tested-by: Daniel Yang <danielyangkang@xxxxxxxxx> Reported-by: syzbot+e953a8f3071f5c0a28fd@xxxxxxxxxxxxxxxxxxxxxxxxx Closes: https://syzkaller.appspot.com/bug?extid=e953a8f3071f5c0a28fd --- net/smc/af_smc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index 0316217b7..e04f132be 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -895,11 +895,15 @@ static void smc_fback_replace_callbacks(struct smc_sock *smc) write_unlock_bh(&clcsk->sk_callback_lock); } -static int smc_switch_to_fallback(struct smc_sock *smc, int reason_code) +/* assumes smc->clcsock_release_lock is held during execution + * reason for separating locking is to give flexibility in + * lock ordering in functions wanting to call smc_switch_to_fallback + * so that deadlocks can be avoided. + */ +static inline int __smc_switch_to_fallback(struct smc_sock *smc, int reason_code) { int rc = 0; - mutex_lock(&smc->clcsock_release_lock); if (!smc->clcsock) { rc = -EBADF; goto out; @@ -923,6 +927,13 @@ static int smc_switch_to_fallback(struct smc_sock *smc, int reason_code) smc_fback_replace_callbacks(smc); } out: + return rc; +} + +static int smc_switch_to_fallback(struct smc_sock *smc, int reason_code) +{ + mutex_lock(&smc->clcsock_release_lock); + int rc = __smc_switch_to_fallback(smc, reason_code); mutex_unlock(&smc->clcsock_release_lock); return rc; } @@ -2762,13 +2773,15 @@ int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) int rc; smc = smc_sk(sk); + /* acquire smc lock before sk to avoid deadlock with rtnl */ + mutex_lock(&smc->clcsock_release_lock); lock_sock(sk); /* SMC does not support connect with fastopen */ if (msg->msg_flags & MSG_FASTOPEN) { /* not connected yet, fallback */ if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) { - rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); + rc = __smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); if (rc) goto out; } else { @@ -2790,6 +2803,7 @@ int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) } out: release_sock(sk); + mutex_unlock(&smc->clcsock_release_lock); return rc; } -- 2.39.2
