public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: Race condition between driver_probe_device and device_shutdown‏
@ 2012-05-18  4:59 Greg KH
  2012-05-20 14:08 ` Ming Lei
  0 siblings, 1 reply; 24+ messages in thread
From: Greg KH @ 2012-05-18  4:59 UTC (permalink / raw)
  To: Wedson Almeida Filho; +Cc: Andrew Morton, linux-kernel

> Hi,

First off, sorry for missing this, and thanks to Andrew for pointing it
out to me.  You might want to use the tool, scripts/get_maintainer.pl
for who to know to cc: for patches like this, so I don't miss it.

> I'm seeing a driver crash in its shutdown routine because it's
> touching some uninitialized state. It turns out that the driver's
> probe routine was still running [for the same device]. There also
> appears to be an issue in the remove path, where device_shutdown()
> checks the dev->driver pointer and uses it later, with seemingly
> nothing to guarantee that it doesn't change.

What type of driver is having this problem?  What type of bus is it on?
Usually the bus prevents this from happening with its own serialization.

> Shouldn't we synchronize the shutdown routine with probe/remove to
> prevent such races?

Normally, yes, and for some reason, I thought we already were doing
that.

> The patch below should take care of these races.

Does this patch solve your problem?  Care to show me the oops you get
without it?

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index e28ce98..f2c63c6 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1823,6 +1823,9 @@ void device_shutdown(void)
>                 pm_runtime_get_noresume(dev);
>                 pm_runtime_barrier(dev);
> 
> +               if (dev->parent)        /* Needed for USB */
> +                       device_lock(dev->parent);
> +               device_lock(dev);

The parent thing is "interesting", and if we are going to have to start
duplicating this logic in different parts of the driver core, we should
wrap it up in a common function, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Race condition between driver_probe_device and device_shutdown‏
@ 2012-05-10  2:53 Wedson Almeida Filho
  0 siblings, 0 replies; 24+ messages in thread
From: Wedson Almeida Filho @ 2012-05-10  2:53 UTC (permalink / raw)
  To: linux-kernel

Hi,

I'm seeing a driver crash in its shutdown routine because it's
touching some uninitialized state. It turns out that the driver's
probe routine was still running [for the same device]. There also
appears to be an issue in the remove path, where device_shutdown()
checks the dev->driver pointer and uses it later, with seemingly
nothing to guarantee that it doesn't change.

Shouldn't we synchronize the shutdown routine with probe/remove to
prevent such races?

The patch below should take care of these races.

Thoughts?

diff --git a/drivers/base/core.c b/drivers/base/core.c
index e28ce98..f2c63c6 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1823,6 +1823,9 @@ void device_shutdown(void)
                pm_runtime_get_noresume(dev);
                pm_runtime_barrier(dev);

+               if (dev->parent)        /* Needed for USB */
+                       device_lock(dev->parent);
+               device_lock(dev);
                if (dev->bus && dev->bus->shutdown) {
                        dev_dbg(dev, "shutdown\n");
                        dev->bus->shutdown(dev);
@@ -1830,6 +1833,9 @@ void device_shutdown(void)
                        dev_dbg(dev, "shutdown\n");
                        dev->driver->shutdown(dev);
                }
+               device_unlock(dev);
+               if (dev->parent)
+                       device_unlock(dev->parent);
                put_device(dev);

                spin_lock(&devices_kset->list_lock);

^ permalink raw reply related	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-12-07 16:27 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-18  4:59 Race condition between driver_probe_device and device_shutdown‏ Greg KH
2012-05-20 14:08 ` Ming Lei
2012-05-20 19:51   ` Greg KH
2012-05-21  4:53     ` Ming Lei
2012-05-21 18:29       ` Alan Stern
2012-05-22  0:35         ` Ming Lei
2012-05-22 14:11           ` Alan Stern
2012-05-22 19:16             ` Eric W. Biederman
2012-05-23 10:05               ` Ming Lei
2012-05-23 15:06                 ` Alan Stern
2012-05-24  1:39                   ` Ming Lei
2012-05-24  2:14                     ` Greg KH
2012-05-24  3:12                       ` Ming Lei
2012-05-24 14:37                       ` Alan Stern
2012-05-25  0:33                         ` Ming Lei
2012-12-06  9:11                           ` Race condition between driver_probe_device and device_shutdown Wedson Almeida Filho
2012-12-06 10:52                             ` Ming Lei
2012-12-07  0:09                               ` Wedson Almeida Filho
2012-12-07  3:42                                 ` Ming Lei
2012-12-07 12:16                                   ` Wedson Almeida Filho
2012-12-07 13:04                                     ` Ming Lei
2012-12-07 15:25                                       ` Alan Stern
2012-12-07 16:27                                         ` Ming Lei
  -- strict thread matches above, loose matches on Subject: below --
2012-05-10  2:53 Race condition between driver_probe_device and device_shutdown‏ Wedson Almeida Filho

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox