From: Rafael J. Wysocki Date: Mon, 3 Feb 2014 23:43:05 +0000 (+0100) Subject: ACPI / hotplug: Fix potential race in acpi_bus_notify() X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=78ea4639a7647f2fcc957c3a532bde49df9895c7;p=deliverable%2Flinux.git ACPI / hotplug: Fix potential race in acpi_bus_notify() There is a slight possibility for the ACPI device object pointed to by adev in acpi_hotplug_notify_cb() to become invalid between the acpi_bus_get_device() that it comes from and the subsequent dereference of that pointer under get_device(). Namely, if acpi_scan_drop_device() runs in parallel with acpi_hotplug_notify_cb(), acpi_device_del_work_fn() queued up by it may delete the device object in question right after a successful execution of acpi_bus_get_device() in acpi_bus_notify(). An analogous problem is present in acpi_bus_notify() where the device pointer coming from acpi_bus_get_device() may become invalid before it subsequent dereference in the "if" block. To prevent that from happening, introduce a new function, acpi_bus_get_acpi_device(), working analogously to acpi_bus_get_device() except that it will grab a reference to the ACPI device object returned by it and it will do that under the ACPICA's namespace mutex. Then, make both acpi_hotplug_notify_cb() and acpi_bus_notify() use acpi_bus_get_acpi_device() instead of acpi_bus_get_device() so as to ensure that the pointers used by them will not become stale at one point. In addition to that, introduce acpi_bus_put_acpi_device() as a wrapper around put_device() to be used along with acpi_bus_get_acpi_device() and make the (new) users of the latter use acpi_bus_put_acpi_device() too. Signed-off-by: Rafael J. Wysocki Tested-by: Mika Westerberg --- diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index fcb59c21c68d..c3237774632f 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -340,7 +340,7 @@ static void acpi_bus_osc_support(void) */ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data) { - struct acpi_device *device = NULL; + struct acpi_device *device; struct acpi_driver *driver; ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Notification %#02x to handle %p\n", @@ -387,12 +387,14 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data) break; } - acpi_bus_get_device(handle, &device); + device = acpi_bus_get_acpi_device(handle); if (device) { driver = device->driver; if (driver && driver->ops.notify && (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS)) driver->ops.notify(device, type); + + acpi_bus_put_acpi_device(device); } } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 7384158c7f87..59eba29a6066 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -476,7 +476,7 @@ static void acpi_device_hotplug(void *data, u32 src) out: acpi_evaluate_hotplug_ost(adev->handle, src, ost_code, NULL); - put_device(&adev->dev); + acpi_bus_put_acpi_device(adev); mutex_unlock(&acpi_scan_lock); unlock_device_hotplug(); } @@ -488,9 +488,6 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data) struct acpi_device *adev; acpi_status status; - if (acpi_bus_get_device(handle, &adev)) - goto err_out; - switch (type) { case ACPI_NOTIFY_BUS_CHECK: acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n"); @@ -512,12 +509,15 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data) /* non-hotplug event; possibly handled by other handler */ return; } - get_device(&adev->dev); + adev = acpi_bus_get_acpi_device(handle); + if (!adev) + goto err_out; + status = acpi_hotplug_execute(acpi_device_hotplug, adev, type); if (ACPI_SUCCESS(status)) return; - put_device(&adev->dev); + acpi_bus_put_acpi_device(adev); err_out: acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL); @@ -1112,14 +1112,16 @@ static void acpi_scan_drop_device(acpi_handle handle, void *context) mutex_unlock(&acpi_device_del_lock); } -int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device) +static int acpi_get_device_data(acpi_handle handle, struct acpi_device **device, + void (*callback)(void *)) { acpi_status status; if (!device) return -EINVAL; - status = acpi_get_data(handle, acpi_scan_drop_device, (void **)device); + status = acpi_get_data_full(handle, acpi_scan_drop_device, + (void **)device, callback); if (ACPI_FAILURE(status) || !*device) { ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No context for object [%p]\n", handle)); @@ -1127,8 +1129,32 @@ int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device) } return 0; } + +int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device) +{ + return acpi_get_device_data(handle, device, NULL); +} EXPORT_SYMBOL(acpi_bus_get_device); +static void get_acpi_device(void *dev) +{ + if (dev) + get_device(&((struct acpi_device *)dev)->dev); +} + +struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle) +{ + struct acpi_device *adev = NULL; + + acpi_get_device_data(handle, &adev, get_acpi_device); + return adev; +} + +void acpi_bus_put_acpi_device(struct acpi_device *adev) +{ + put_device(&adev->dev); +} + int acpi_device_add(struct acpi_device *device, void (*release)(struct device *)) { diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 8256eb4ad057..94fd61a0456d 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -381,6 +381,8 @@ extern int unregister_acpi_notifier(struct notifier_block *); */ int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device); +struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle); +void acpi_bus_put_acpi_device(struct acpi_device *adev); acpi_status acpi_bus_get_status_handle(acpi_handle handle, unsigned long long *sta); int acpi_bus_get_status(struct acpi_device *device);