Re: [PATCH v3 2/5] ACPI / bus: Do not traverse through non-existed device table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 8, 2018 at 5:53 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> 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?

Yes, it would.

> --- 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 linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux