Re: [syzbot] [kernfs?] possible deadlock in __kernfs_remove

[Dan Williams <dan.j.williams@intel.com>]

Tetsuo Handa wrote:
> This dependency is added by recent bug fix commit c0a40097f0bc ("drivers:
> core: synchronize really_probe() and dev_uevent()").
> 
> Commit 4a0079bc7aae ("nvdimm: Replace lockdep_mutex with local lock
> classes") changed to assign nvdimm_namespace_key on dev->mutex instead of
> __lockdep_no_validate__, which made lockdep to report this dependency.

At first glance this looks like a true positive lockdep report. One
observation is that attributes that depend on dev->driver being stable
are typically more appropriate as a driver lifetime scoped attribute
rather than adding locking within the attribute handler.

So a rough idea for an alternate fix would be something like:

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 83d352394fdf..a0536e6eaea3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -672,6 +672,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
                goto probe_failed;
        }
 
+       device_remove_file(dev, &dev_attr_uevent);
+       device_create_file(dev, &dev_attr_driver_uevent);
        ret = device_add_groups(dev, drv->dev_groups);
        if (ret) {
                dev_err(dev, "device_add_groups() failed\n");

...where the only difference between those 2 attributes is that
dev_attr_driver_uevent additionally appends the "DRIVER=" key to the
uevent.

The problem with the above though is userspace may not expect that it
needs to re-open the uevent file over a driver attach / detach event.

Either way I think c0a40097f0bc ("drivers: core: synchronize
really_probe() and dev_uevent()") is potentially more of a band-aid than
a fix.