On Thu, 2018-02-08 at 16:59 +0100, Rafael J. Wysocki wrote: > On Wed, Feb 7, 2018 at 3:56 PM, Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > And as far as I'm concerned you can do: > > if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id) > return (const struct acpi_device_id > *)acpi_of_match_device(device, of_ids); > > and update the comment accordingly. Would the following work for you? --- 8< --- 8< --- 8< --- >From 03013c0c1e487cda4c0d8dc5065ed2f0031095de Mon Sep 17 00:00:00 2001 From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Date: Thu, 1 Feb 2018 21:52:14 +0200 Subject: [PATCH 1/1] ACPI / bus: Do not traverse through non-existed device table When __acpi_match_device() is called it would be possible to have ACPI ID table a MULL pointer. To avoid potential dereference, check for this before traverse. This patch implies a bit of refactoring acpi_of_match_device() to return pointer to OF ID when matched followed by refactoring __acpi_match_device() to return either ACPI or OF ID when matches. While here, remove redundant 'else'. Cc: Sinan Kaya <okaya@xxxxxxxxxxxxxx> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> Cc: Vinod Koul <vinod.koul@xxxxxxxxx> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> --- drivers/acpi/bus.c | 66 +++++++++++++++++++++++++++++++++------------ --------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 676c9788e1c8..33c5166e6028 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -660,13 +660,15 @@ struct acpi_device *acpi_companion_match(const struct device *dev) * acpi_of_match_device - Match device object using the "compatible" property. * @adev: ACPI device object to match. * @of_match_table: List of device IDs to match against. + * @of_id: OF ID if matched * * If @dev has an ACPI companion which has ACPI_DT_NAMESPACE_HID in its list of * identifiers and a _DSD object with the "compatible" property, use that * property to match against the given list of identifiers. */ static bool acpi_of_match_device(struct acpi_device *adev, - const struct of_device_id *of_match_table) + const struct of_device_id *of_match_table, + const struct of_device_id **of_id) { const union acpi_object *of_compatible, *obj; int i, nval; @@ -690,8 +692,11 @@ static bool acpi_of_match_device(struct acpi_device *adev, const struct of_device_id *id; for (id = of_match_table; id->compatible[0]; id++) - if (!strcasecmp(obj->string.pointer, id- >compatible)) + if (!strcasecmp(obj->string.pointer, id- >compatible)) { + if (of_id) + *of_id = id; return true; + } } return false; @@ -762,10 +767,11 @@ static bool __acpi_match_device_cls(const struct acpi_device_id *id, return true; } -static const struct acpi_device_id *__acpi_match_device( - struct acpi_device *device, - const struct acpi_device_id *ids, - const struct of_device_id *of_ids) +static bool __acpi_match_device(struct acpi_device *device, + const struct acpi_device_id *acpi_ids, + const struct of_device_id *of_ids, + const struct acpi_device_id **acpi_id, + const struct of_device_id **of_id) { const struct acpi_device_id *id; struct acpi_hardware_id *hwid; @@ -775,30 +781,35 @@ static const struct acpi_device_id *__acpi_match_device( * driver for it. */ if (!device || !device->status.present) - return NULL; + return false; list_for_each_entry(hwid, &device->pnp.ids, list) { /* First, check the ACPI/PNP IDs provided by the caller. */ - for (id = ids; id->id[0] || id->cls; id++) { - if (id->id[0] && !strcmp((char *) id->id, hwid- >id)) - return id; - else if (id->cls && __acpi_match_device_cls(id, hwid)) - return id; + if (acpi_ids) { + bool found; + + for (id = acpi_ids; id->id[0] || id->cls; id++) { + found = false; + if (id->id[0] && !strcmp((char *)id- >id, hwid->id)) + found = true; + if (id->cls && __acpi_match_device_cls(id, hwid)) + found = true; + if (found) { + if (acpi_id) + *acpi_id = id; + return true; + } + } } /* * Next, check ACPI_DT_NAMESPACE_HID and try to match the * "compatible" property if found. - * - * The id returned by the below is not valid, but the only - * caller passing non-NULL of_ids here is only interested in - * whether or not the return value is NULL. */ - if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id) - && acpi_of_match_device(device, of_ids)) - return id; + if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)) + return acpi_of_match_device(device, of_ids, of_id); } - return NULL; + return false; } /** @@ -815,7 +826,10 @@ static const struct acpi_device_id *__acpi_match_device( const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids, const struct device *dev) { - return __acpi_match_device(acpi_companion_match(dev), ids, NULL); + const struct acpi_device_id *id = NULL; + + __acpi_match_device(acpi_companion_match(dev), ids, NULL, &id, NULL); + return id; } EXPORT_SYMBOL_GPL(acpi_match_device); @@ -840,7 +854,7 @@ EXPORT_SYMBOL_GPL(acpi_get_match_data); int acpi_match_device_ids(struct acpi_device *device, const struct acpi_device_id *ids) { - return __acpi_match_device(device, ids, NULL) ? 0 : -ENOENT; + return __acpi_match_device(device, ids, NULL, NULL, NULL) ? 0 : -ENOENT; } EXPORT_SYMBOL(acpi_match_device_ids); @@ -849,10 +863,12 @@ bool acpi_driver_match_device(struct device *dev, { if (!drv->acpi_match_table) return acpi_of_match_device(ACPI_COMPANION(dev), - drv->of_match_table); + drv->of_match_table, + NULL); - return !!__acpi_match_device(acpi_companion_match(dev), - drv->acpi_match_table, drv- >of_match_table); + return __acpi_match_device(acpi_companion_match(dev), + drv->acpi_match_table, drv- >of_match_table, + NULL, NULL); } EXPORT_SYMBOL_GPL(acpi_driver_match_device); -- 2.15.1 -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html