ACPI: platform-profile: Fix possible deadlock in platform_profile_remove()
authorHans de Goede <hdegoede@redhat.com>
Mon, 25 Jan 2021 19:09:09 +0000 (20:09 +0100)
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>
Wed, 27 Jan 2021 17:48:12 +0000 (18:48 +0100)
After a rmmod thinkpad_acpi, lockdep pointed out this possible deadlock:

Our _show and _store sysfs attr functions get called with the kn->active
lock held for the sysfs attr and then take the profile_lock.
sysfs_remove_group() also takes the kn->active lock for the sysfs attr,
so if we call it with the profile_lock held, then we get an ABBA deadlock.

platform_profile_remove() must only be called by drivers which have
first *successfully* called platform_profile_register(). Anything else
is a driver bug. So the check for cur_profile being set before calling
sysfs_remove_group() is not necessary and it can be dropped.

It is safe to call sysfs_remove_group() without holding the profile_lock
since the attr-group group cannot be re-added until after we clear
cur_profile.

Change platform_profile_remove() to only hold the profile_lock while
clearing the cur_profile, fixing the deadlock.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
drivers/acpi/platform_profile.c

index 80e9df427eb8eaf7b93de35b7d562332c65f4a5d..4a59c5993bde432b482f85d6bf7c345b6467b3dd 100644 (file)
@@ -164,13 +164,9 @@ EXPORT_SYMBOL_GPL(platform_profile_register);
 
 int platform_profile_remove(void)
 {
-       mutex_lock(&profile_lock);
-       if (!cur_profile) {
-               mutex_unlock(&profile_lock);
-               return -ENODEV;
-       }
-
        sysfs_remove_group(acpi_kobj, &platform_profile_group);
+
+       mutex_lock(&profile_lock);
        cur_profile = NULL;
        mutex_unlock(&profile_lock);
        return 0;