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] [day] [month] [year] [list]

Date:   Wed, 22 Sep 2021 19:07:56 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        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,
        syzkaller-bugs@...glegroups.com,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [syzbot] possible deadlock in mptcp_close

On Wed, 2021-09-22 at 17:57 +0200, Thomas Gleixner wrote:
> 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.

Thank you for looking into this! I agree this specific case is not
fastpath, so definitely the proposed patch LGTM.

I fear there could be other similar cases in the MPTCP code, in more
time critical paths, and perhaps there are other relevant use-case, so
I'd like to experiment too with a lock_sock_fast_nested() variant - if
I find enough coffee ;)

Thanks,

Paolo