• Dan Williams's avatar
    driver core: Fix uevent_show() vs driver detach race · 15fffc6a
    Dan Williams authored
    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: c0a40097 ("drivers: core: synchronize really_probe() and dev_uevent()")
    Reported-by: syzbot+4762dd74e32532cda5ff@syzkaller.appspotmail.com
    Reported-by: default avatarTetsuo 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: default avatarDan Williams <dan.j.williams@intel.com>
    Link: https://lore.kernel.org/r/172081332794.577428.9738802016494057132.stgit@dwillia2-xfh.jf.intel.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    15fffc6a
module.c 2.33 KB