Skip to content

Commit

Permalink
ACPI: utils: Fix reference counting in for_each_acpi_dev_match()
Browse files Browse the repository at this point in the history
Currently it's possible to iterate over the dangling pointer in case the device
suddenly disappears. This may happen becase callers put it at the end of a loop.

Instead, let's move that call inside acpi_dev_get_next_match_dev().

Fixes: 803abec ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Fixes: bf263f6 ("media: ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro")
Fixes: edbd1bc ("efi/dev-path-parser: Switch to use for_each_acpi_dev_match()")
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Daniel Scally <djrscally@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
andy-shev authored and rafaeljw committed Jul 19, 2021
1 parent 2734d6c commit 71f6428
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 14 deletions.
7 changes: 3 additions & 4 deletions drivers/acpi/utils.c
Expand Up @@ -860,11 +860,9 @@ EXPORT_SYMBOL(acpi_dev_present);
* Return the next match of ACPI device if another matching device was present
* at the moment of invocation, or NULL otherwise.
*
* FIXME: The function does not tolerate the sudden disappearance of @adev, e.g.
* in the case of a hotplug event. That said, the caller should ensure that
* this will never happen.
*
* The caller is responsible for invoking acpi_dev_put() on the returned device.
* On the other hand the function invokes acpi_dev_put() on the given @adev
* assuming that its reference counter had been increased beforehand.
*
* See additional information in acpi_dev_present() as well.
*/
Expand All @@ -880,6 +878,7 @@ acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const cha
match.hrv = hrv;

dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
acpi_dev_put(adev);
return dev ? to_acpi_device(dev) : NULL;
}
EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
Expand Down
1 change: 0 additions & 1 deletion drivers/firmware/efi/dev-path-parser.c
Expand Up @@ -34,7 +34,6 @@ static long __init parse_acpi_path(const struct efi_dev_path *node,
break;
if (!adev->pnp.unique_id && node->acpi.uid == 0)
break;
acpi_dev_put(adev);
}
if (!adev)
return -ENODEV;
Expand Down
6 changes: 2 additions & 4 deletions drivers/media/pci/intel/ipu3/cio2-bridge.c
Expand Up @@ -173,10 +173,8 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
int ret;

for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
if (!adev->status.enabled) {
acpi_dev_put(adev);
if (!adev->status.enabled)
continue;
}

if (bridge->n_sensors >= CIO2_NUM_PORTS) {
acpi_dev_put(adev);
Expand All @@ -185,7 +183,6 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
}

sensor = &bridge->sensors[bridge->n_sensors];
sensor->adev = adev;
strscpy(sensor->name, cfg->hid, sizeof(sensor->name));

ret = cio2_bridge_read_acpi_buffer(adev, "SSDB",
Expand Down Expand Up @@ -215,6 +212,7 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
goto err_free_swnodes;
}

sensor->adev = acpi_dev_get(adev);
adev->fwnode.secondary = fwnode;

dev_info(&cio2->dev, "Found supported sensor %s\n",
Expand Down
5 changes: 0 additions & 5 deletions include/acpi/acpi_bus.h
Expand Up @@ -707,11 +707,6 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
* @hrv: Hardware Revision of the device, pass -1 to not check _HRV
*
* The caller is responsible for invoking acpi_dev_put() on the returned device.
*
* FIXME: Due to above requirement there is a window that may invalidate @adev
* and next iteration will use a dangling pointer, e.g. in the case of a
* hotplug event. That said, the caller should ensure that this will never
* happen.
*/
#define for_each_acpi_dev_match(adev, hid, uid, hrv) \
for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv); \
Expand Down

0 comments on commit 71f6428

Please sign in to comment.