Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 14 Jul 2018 14:57:49 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Dmitry Torokhov <dtor@chromium.org>
Cc:     toddpoynor@gmail.com, rspringer@google.com, jnjoseph@google.com,
        benchan@chromium.org, frankhu@chromium.org,
        Simon Que <sque@chromium.org>,
        Guenter Roeck <groeck@chromium.org>,
        devel@driverdev.osuosl.org, lkml <linux-kernel@vger.kernel.org>,
        toddpoynor@google.com
Subject: Re: [PATCH 06/18] staging: gasket: fix deadlock in pci driver
 unregister path
Message-ID: <20180714125749.GA19498@kroah.com>
References: <20180714055816.223754-1-toddpoynor@gmail.com>
 <20180714055816.223754-7-toddpoynor@gmail.com>
 <CAE_wzQ9BCLryFUrytYEJ+tSsog4nUXEr4cbtYNgbq3w7U-qezQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAE_wzQ9BCLryFUrytYEJ+tSsog4nUXEr4cbtYNgbq3w7U-qezQ@mail.gmail.com>
User-Agent: Mutt/1.10.0 (2018-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@gmail.com> wrote:
> >
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > g_mutex held across pci_unregister_driver() call, also held in
> > gasket_pci_remove(), which deadlocks.
> >
> > Reported-by: Dmitry Torokhov <dtor@chromium.org>
> > Signed-off-by: Zhongze Hu <frankhu@chromium.org>
> > Signed-off-by: Todd Poynor <toddpoynor@google.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