netdev - Re: [syzbot] possible deadlock in mptcp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87zgs4habc.ffs@tglx>
Date:   Wed, 22 Sep 2021 17:57:59 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     syzbot <syzbot+1dd53f7a89b299d59eaf@...kaller.appspotmail.com>,
        davem@...emloft.net, edumazet@...gle.com, fw@...len.de,
        kuba@...nel.org, linux-kernel@...r.kernel.org,
        mathew.j.martineau@...ux.intel.com, matthieu.baerts@...sares.net,
        mptcp@...ts.linux.dev, netdev@...r.kernel.org, pabeni@...hat.com,
        syzkaller-bugs@...glegroups.com,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [syzbot] possible deadlock in mptcp_close

On Mon, Sep 20 2021 at 15:04, syzbot wrote:
> The issue was bisected to:
>
> commit 2dcb96bacce36021c2f3eaae0cef607b5bb71ede
> Author: Thomas Gleixner <tglx@...utronix.de>
> Date:   Sat Sep 18 12:42:35 2021 +0000
>
>     net: core: Correct the sock::sk_lock.owned lockdep annotations

Shooting the messenger...

> MPTCP: kernel_bind error, err=-98
> ============================================
> WARNING: possible recursive locking detected
> 5.15.0-rc1-syzkaller #0 Not tainted
> --------------------------------------------
> syz-executor998/6520 is trying to acquire lock:
> ffff8880795718a0 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x267/0x7b0 net/mptcp/protocol.c:2738
>
> but task is already holding lock:
> ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1612 [inline]
> ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x23/0x7b0 net/mptcp/protocol.c:2720

So this is a lock nesting issue and looking at the stack trace this
comes from:

>  lock_sock_fast+0x36/0x100 net/core/sock.c:3229

which does not support lockdep nesting. So from a lockdep POV this is
recursive locking the same lock class. And it's the case I was worried
about that lockdep testing never takes the slow path. The original
lockdep annotation would have produced exactly the same splat in the
slow path case.

So it's not a new problem. It's just visible by moving the lockdep
annotations to a place where they actually can detect issues which were
not reported before.

See also https://lore.kernel.org/lkml/874kacu248.ffs@tglx/

There are two ways to address this mptcp one:

  1) Teach lock_sock_fast() about lock nesting

  2) Use lock_sock_nested() in mptcp_close() as that should not be
     really a hotpath. See patch below.

Thanks,

        tglx
---

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 2602f1386160..27ea5d4dfdf6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2735,10 +2735,10 @@ static void mptcp_close(struct sock *sk, long timeout)
 	inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32;
 	mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
-		bool slow = lock_sock_fast(ssk);
 
+		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
 		sock_orphan(ssk);
-		unlock_sock_fast(ssk, slow);
+		unlock_sock(ssk);
 	}
 	sock_orphan(sk);