linux-kernel - Re: [PATCH] tty: n_gsm: avoid recursive locking with async port hangup

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: <20190904071740.GE18791@kroah.com>
Date:   Wed, 4 Sep 2019 09:17:40 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Martin Hundebøll <martin@...nix.com>
Cc:     Jiri Slaby <jslaby@...e.com>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        linux-kernel@...r.kernel.org,
        Sean Nyekjær <sean@...nix.com>,
        Esben Haabendal <esben@...nix.com>
Subject: Re: [PATCH] tty: n_gsm: avoid recursive locking with async port
 hangup

On Thu, Aug 29, 2019 at 09:42:31PM +0200, Martin Hundebøll wrote:
> On 22/08/2019 23.56, Martin Hundebøll wrote:
> > When tearing down the n_gsm ldisc while one or more of its child ports
> > are open, a lock dep warning occurs:
> > 
> > [   56.254258] ======================================================
> > [   56.260447] WARNING: possible circular locking dependency detected
> > [   56.266641] 5.2.0-00118-g1fd58e20e5b0 #30 Not tainted
> > [   56.271701] ------------------------------------------------------
> > [   56.277890] cmux/271 is trying to acquire lock:
> > [   56.282436] 8215283a (&tty->legacy_mutex){+.+.}, at: __tty_hangup.part.0+0x58/0x27c
> > [   56.290128]
> > [   56.290128] but task is already holding lock:
> > [   56.295970] e9e2b842 (&gsm->mutex){+.+.}, at: gsm_cleanup_mux+0x9c/0x15c
> > [   56.302699]
> > [   56.302699] which lock already depends on the new lock.
> > [   56.302699]
> > [   56.310884]
> > [   56.310884] the existing dependency chain (in reverse order) is:
> > [   56.318372]
> > [   56.318372] -> #2 (&gsm->mutex){+.+.}:
> > [   56.323624]        mutex_lock_nested+0x1c/0x24
> > [   56.328079]        gsm_cleanup_mux+0x9c/0x15c
> > [   56.332448]        gsmld_ioctl+0x418/0x4e8
> > [   56.336554]        tty_ioctl+0x96c/0xcb0
> > [   56.340492]        do_vfs_ioctl+0x41c/0xa5c
> > [   56.344685]        ksys_ioctl+0x34/0x60
> > [   56.348535]        ret_fast_syscall+0x0/0x28
> > [   56.352815]        0xbe97cc04
> > [   56.355791]
> > [   56.355791] -> #1 (&tty->ldisc_sem){++++}:
> > [   56.361388]        tty_ldisc_lock+0x50/0x74
> > [   56.365581]        tty_init_dev+0x88/0x1c4
> > [   56.369687]        tty_open+0x1c8/0x430
> > [   56.373536]        chrdev_open+0xa8/0x19c
> > [   56.377560]        do_dentry_open+0x118/0x3c4
> > [   56.381928]        path_openat+0x2fc/0x1190
> > [   56.386123]        do_filp_open+0x68/0xd4
> > [   56.390146]        do_sys_open+0x164/0x220
> > [   56.394257]        kernel_init_freeable+0x328/0x3e4
> > [   56.399146]        kernel_init+0x8/0x110
> > [   56.403078]        ret_from_fork+0x14/0x20
> > [   56.407183]        0x0
> > [   56.409548]
> > [   56.409548] -> #0 (&tty->legacy_mutex){+.+.}:
> > [   56.415402]        __mutex_lock+0x64/0x90c
> > [   56.419508]        mutex_lock_nested+0x1c/0x24
> > [   56.423961]        __tty_hangup.part.0+0x58/0x27c
> > [   56.428676]        gsm_cleanup_mux+0xe8/0x15c
> > [   56.433043]        gsmld_close+0x48/0x90
> > [   56.436979]        tty_ldisc_kill+0x2c/0x6c
> > [   56.441173]        tty_ldisc_release+0x88/0x194
> > [   56.445715]        tty_release_struct+0x14/0x44
> > [   56.450254]        tty_release+0x36c/0x43c
> > [   56.454365]        __fput+0x94/0x1e8
> > 
> > Avoid the warning by doing the port hangup asynchronously.
> 
> Any comments?

Sorry, was waiting to see if anyone would review this, I'll go do it
later today...