linux-kernel - Re: [PATCH 06/18] staging: gasket: fix deadlock in pci driver unregister path

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 14 Jul 2018 14:57:49 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Dmitry Torokhov <dtor@...omium.org>
Cc:     toddpoynor@...il.com, rspringer@...gle.com, jnjoseph@...gle.com,
        benchan@...omium.org, frankhu@...omium.org,
        Simon Que <sque@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        devel@...verdev.osuosl.org, lkml <linux-kernel@...r.kernel.org>,
        toddpoynor@...gle.com
Subject: Re: [PATCH 06/18] staging: gasket: fix deadlock in pci driver
 unregister path

On Sat, Jul 14, 2018 at 11:07:21AM +0300, Dmitry Torokhov wrote:
> On Sat, Jul 14, 2018 at 8:58 AM Todd Poynor <toddpoynor@...il.com> wrote:
> >
> > From: Todd Poynor <toddpoynor@...gle.com>
> >
> > g_mutex held across pci_unregister_driver() call, also held in
> > gasket_pci_remove(), which deadlocks.
> >
> > Reported-by: Dmitry Torokhov <dtor@...omium.org>
> > Signed-off-by: Zhongze Hu <frankhu@...omium.org>
> > Signed-off-by: Todd Poynor <toddpoynor@...gle.com>
> > ---
> >  drivers/staging/gasket/gasket_core.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
> > index 3bdf7d36b397..6d240dc59557 100644
> > --- a/drivers/staging/gasket/gasket_core.c
> > +++ b/drivers/staging/gasket/gasket_core.c
> > @@ -668,13 +668,10 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)
> >         struct gasket_dev *gasket_dev = NULL;
> >         const struct gasket_driver_desc *driver_desc;
> >         /* Find the device desc. */
> > -       mutex_lock(&g_mutex);
> > +       __must_hold(&g_mutex);
> 
> And what exactly ensures that mutex is held here? Yes, we are holding
> the mutex when we unload the driver, but PCI hot-unplug or unbinding
> the device though sysfs do not go through module unload code path, so
> you'll end up here without holding the mutex.

Which is a huge reason the whole "wrap the pci core calls" is not going
to work here at all.  The device ownership rules are all wonky because
of this.  Unwinding that is key to getting all of this right.

thanks,

greg k-h