1ee9fd53e6
commit 15fffc6a5624b13b428bb1c6e9088e32a55eb82c upstream. uevent_show() wants to de-reference dev->driver->name. There is no clean way for a device attribute to de-reference dev->driver unless that attribute is defined via (struct device_driver).dev_groups. Instead, the anti-pattern of taking the device_lock() in the attribute handler risks deadlocks with code paths that remove device attributes while holding the lock. This deadlock is typically invisible to lockdep given the device_lock() is marked lockdep_set_novalidate_class(), but some subsystems allocate a local lockdep key for @dev->mutex to reveal reports of the form: ====================================================== WARNING: possible circular locking dependency detected 6.10.0-rc7+ #275 Tainted: G OE N ------------------------------------------------------ modprobe/2374 is trying to acquire lock: ffff8c2270070de0 (kn->active#6){++++}-{0:0}, at: __kernfs_remove+0xde/0x220 but task is already holding lock: ffff8c22016e88f8 (&cxl_root_key){+.+.}-{3:3}, at: device_release_driver_internal+0x39/0x210 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&cxl_root_key){+.+.}-{3:3}: __mutex_lock+0x99/0xc30 uevent_show+0xac/0x130 dev_attr_show+0x18/0x40 sysfs_kf_seq_show+0xac/0xf0 seq_read_iter+0x110/0x450 vfs_read+0x25b/0x340 ksys_read+0x67/0xf0 do_syscall_64+0x75/0x190 entry_SYSCALL_64_after_hwframe+0x76/0x7e -> #0 (kn->active#6){++++}-{0:0}: __lock_acquire+0x121a/0x1fa0 lock_acquire+0xd6/0x2e0 kernfs_drain+0x1e9/0x200 __kernfs_remove+0xde/0x220 kernfs_remove_by_name_ns+0x5e/0xa0 device_del+0x168/0x410 device_unregister+0x13/0x60 devres_release_all+0xb8/0x110 device_unbind_cleanup+0xe/0x70 device_release_driver_internal+0x1c7/0x210 driver_detach+0x47/0x90 bus_remove_driver+0x6c/0xf0 cxl_acpi_exit+0xc/0x11 [cxl_acpi] __do_sys_delete_module.isra.0+0x181/0x260 do_syscall_64+0x75/0x190 entry_SYSCALL_64_after_hwframe+0x76/0x7e The observation though is that driver objects are typically much longer lived than device objects. It is reasonable to perform lockless de-reference of a @driver pointer even if it is racing detach from a device. Given the infrequency of driver unregistration, use synchronize_rcu() in module_remove_driver() to close any potential races. It is potentially overkill to suffer synchronize_rcu() just to handle the rare module removal racing uevent_show() event. Thanks to Tetsuo Handa for the debug analysis of the syzbot report [1]. Fixes: c0a40097f0bc ("drivers: core: synchronize really_probe() and dev_uevent()") Reported-by: syzbot+4762dd74e32532cda5ff@syzkaller.appspotmail.com Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Closes: http://lore.kernel.org/5aa5558f-90a4-4864-b1b1-5d6784c5607d@I-love.SAKURA.ne.jp [1] Link: http://lore.kernel.org/669073b8ea479_5fffa294c1@dwillia2-xfh.jf.intel.com.notmuch Cc: stable@vger.kernel.org Cc: Ashish Sangwan <a.sangwan@samsung.com> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Dirk Behme <dirk.behme@de.bosch.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Rafael J. Wysocki <rafael@kernel.org> Signed-off-by: Dan Williams <dan.j.williams@intel.com> Link: https://lore.kernel.org/r/172081332794.577428.9738802016494057132.stgit@dwillia2-xfh.jf.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
97 lines
2.1 KiB
C
Executable file
97 lines
2.1 KiB
C
Executable file
// SPDX-License-Identifier: GPL-2.0
|
|
/*
|
|
* module.c - module sysfs fun for drivers
|
|
*/
|
|
#include <linux/device.h>
|
|
#include <linux/module.h>
|
|
#include <linux/errno.h>
|
|
#include <linux/slab.h>
|
|
#include <linux/string.h>
|
|
#include <linux/rcupdate.h>
|
|
#include "base.h"
|
|
|
|
static char *make_driver_name(struct device_driver *drv)
|
|
{
|
|
char *driver_name;
|
|
|
|
driver_name = kasprintf(GFP_KERNEL, "%s:%s", drv->bus->name, drv->name);
|
|
if (!driver_name)
|
|
return NULL;
|
|
|
|
return driver_name;
|
|
}
|
|
|
|
static void module_create_drivers_dir(struct module_kobject *mk)
|
|
{
|
|
static DEFINE_MUTEX(drivers_dir_mutex);
|
|
|
|
mutex_lock(&drivers_dir_mutex);
|
|
if (mk && !mk->drivers_dir)
|
|
mk->drivers_dir = kobject_create_and_add("drivers", &mk->kobj);
|
|
mutex_unlock(&drivers_dir_mutex);
|
|
}
|
|
|
|
void module_add_driver(struct module *mod, struct device_driver *drv)
|
|
{
|
|
char *driver_name;
|
|
int no_warn;
|
|
struct module_kobject *mk = NULL;
|
|
|
|
if (!drv)
|
|
return;
|
|
|
|
if (mod)
|
|
mk = &mod->mkobj;
|
|
else if (drv->mod_name) {
|
|
struct kobject *mkobj;
|
|
|
|
/* Lookup built-in module entry in /sys/modules */
|
|
mkobj = kset_find_obj(module_kset, drv->mod_name);
|
|
if (mkobj) {
|
|
mk = container_of(mkobj, struct module_kobject, kobj);
|
|
/* remember our module structure */
|
|
drv->p->mkobj = mk;
|
|
/* kset_find_obj took a reference */
|
|
kobject_put(mkobj);
|
|
}
|
|
}
|
|
|
|
if (!mk)
|
|
return;
|
|
|
|
/* Don't check return codes; these calls are idempotent */
|
|
no_warn = sysfs_create_link(&drv->p->kobj, &mk->kobj, "module");
|
|
driver_name = make_driver_name(drv);
|
|
if (driver_name) {
|
|
module_create_drivers_dir(mk);
|
|
no_warn = sysfs_create_link(mk->drivers_dir, &drv->p->kobj,
|
|
driver_name);
|
|
kfree(driver_name);
|
|
}
|
|
}
|
|
|
|
void module_remove_driver(struct device_driver *drv)
|
|
{
|
|
struct module_kobject *mk = NULL;
|
|
char *driver_name;
|
|
|
|
if (!drv)
|
|
return;
|
|
|
|
/* Synchronize with dev_uevent() */
|
|
synchronize_rcu();
|
|
|
|
sysfs_remove_link(&drv->p->kobj, "module");
|
|
|
|
if (drv->owner)
|
|
mk = &drv->owner->mkobj;
|
|
else if (drv->p->mkobj)
|
|
mk = drv->p->mkobj;
|
|
if (mk && mk->drivers_dir) {
|
|
driver_name = make_driver_name(drv);
|
|
if (driver_name) {
|
|
sysfs_remove_link(mk->drivers_dir, driver_name);
|
|
kfree(driver_name);
|
|
}
|
|
}
|
|
}
|