[BUG] Deferred probing in driver model is racy, resulting in lost probes

On Thu, Sep 27, 2012 at 4:23 AM, Russell King - ARM Linux
[off-list ref] wrote:

On Wed, Sep 26, 2012 at 01:08:33PM -0700, Greg Kroah-Hartman wrote:

quoted

On Sun, Sep 16, 2012 at 09:24:43PM +0800, Ming Lei wrote:

quoted

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 181ed26..17d7437 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c

@@ -714,12 +714,12 @@ int bus_add_driver(struct device_driver *drv)
    if (error)
            goto out_unregister;

+   klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
    if (drv->bus->p->drivers_autoprobe) {
            error = driver_attach(drv);
            if (error)
                    goto out_unregister;
    }
-   klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
    module_add_driver(drv->owner, drv);

    error = driver_create_file(drv, &driver_attr_uevent);

Did the above patch ever prove to solve the issue or not?

To be honest, I've not bothered to test the above patch, and now when I
look at it, I notice it's broken - in that on error it will corrupt the
driver list.  Take a look at the error path.

priv is drv->p.  We add priv->knode_bus to the driver list.  If
driver_attach() returns an error, then we go to out_unregister, which

In fact, driver_attach() always returns zero, so it does __not__ affect
your test.

I knew the failure shouldn't be handled in theory because the probe
failure on one device should not cause failure of driver_register, and
it should be fixed, IMO.

does:

out_unregister:
        kobject_put(&priv->kobj);
        kfree(drv->p);
        drv->p = NULL;

thereby freeing the node we just added to the driver list without first
removing it.

I suspect it will fix the problem, but let's get the patch to be correct
before it gets tested...

Trust me, please go ahead to test and it doesn't affect it...

Thanks,
-- 
Ming Lei