Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

hwmon: (acpi_power_meter) Fix deadlocks related to acpi_power_meter_notify()

The acpi_power_meter driver's .notify() callback function,
acpi_power_meter_notify(), calls hwmon_device_unregister() under a lock
that is also acquired by callbacks in sysfs attributes of the device
being unregistered which is prone to deadlocks between sysfs access and
device removal.

Address this by moving the hwmon device removal in
acpi_power_meter_notify() outside the lock in question, but notice
that doing it alone is not sufficient because two concurrent
METER_NOTIFY_CONFIG notifications may be attempting to remove the
same device at the same time. To prevent that from happening, add a
new lock serializing the execution of the switch () statement in
acpi_power_meter_notify(). For simplicity, it is a static mutex
which should not be a problem from the performance perspective.

The new lock also allows the hwmon_device_register_with_info()
in acpi_power_meter_notify() to be called outside the inner lock
because it prevents the other notifications handled by that function
from manipulating the "resource" object while the hwmon device based
on it is being registered. The sending of ACPI netlink messages from
acpi_power_meter_notify() is serialized by the new lock too which
generally helps to ensure that the order of handling firmware
notifications is the same as the order of sending netlink messages
related to them.

In addition, notice that hwmon_device_register_with_info() may fail
in which case resource->hwmon_dev will become an error pointer,
so add checks to avoid attempting to unregister the hwmon device
pointer to by it in that case to acpi_power_meter_notify() and
acpi_power_meter_remove().

Fixes: 16746ce8adfe ("hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register")
Closes: https://lore.kernel.org/linux-hwmon/CAK8fFZ58fidGUCHi5WFX0uoTPzveUUDzT=k=AAm4yWo3bAuCFg@mail.gmail.com/
Reported-by: Jaroslav Pulchart <jaroslav.pulchart@gooddata.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

authored by

Rafael J. Wysocki and committed by
Guenter Roeck
615901b5 830e0bef

+14 -3
+14 -3
drivers/hwmon/acpi_power_meter.c
··· 47 47 static int cap_in_hardware; 48 48 static bool force_cap_on; 49 49 50 + static DEFINE_MUTEX(acpi_notify_lock); 51 + 50 52 static int can_cap_in_hardware(void) 51 53 { 52 54 return force_cap_on || cap_in_hardware; ··· 825 823 826 824 resource = acpi_driver_data(device); 827 825 826 + guard(mutex)(&acpi_notify_lock); 827 + 828 828 switch (event) { 829 829 case METER_NOTIFY_CONFIG: 830 + if (!IS_ERR(resource->hwmon_dev)) 831 + hwmon_device_unregister(resource->hwmon_dev); 832 + 830 833 mutex_lock(&resource->lock); 834 + 831 835 free_capabilities(resource); 832 836 remove_domain_devices(resource); 833 - hwmon_device_unregister(resource->hwmon_dev); 834 837 res = read_capabilities(resource); 835 838 if (res) 836 839 dev_err_once(&device->dev, "read capabilities failed.\n"); 837 840 res = read_domain_devices(resource); 838 841 if (res && res != -ENODEV) 839 842 dev_err_once(&device->dev, "read domain devices failed.\n"); 843 + 844 + mutex_unlock(&resource->lock); 845 + 840 846 resource->hwmon_dev = 841 847 hwmon_device_register_with_info(&device->dev, 842 848 ACPI_POWER_METER_NAME, ··· 853 843 power_extra_groups); 854 844 if (IS_ERR(resource->hwmon_dev)) 855 845 dev_err_once(&device->dev, "register hwmon device failed.\n"); 856 - mutex_unlock(&resource->lock); 846 + 857 847 break; 858 848 case METER_NOTIFY_TRIP: 859 849 sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME); ··· 963 953 return; 964 954 965 955 resource = acpi_driver_data(device); 966 - hwmon_device_unregister(resource->hwmon_dev); 956 + if (!IS_ERR(resource->hwmon_dev)) 957 + hwmon_device_unregister(resource->hwmon_dev); 967 958 968 959 remove_domain_devices(resource); 969 960 free_capabilities(resource);