| Message ID | 20180201202012.36524-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2/1/2018 3:20 PM, Andy Shevchenko wrote: > When __acpi_match_device() is called it would be possible to have > ACPI ID table a MULL pointer. To avoid potential dereference, NULL > check for this before traverse. > > While here, remove redundant 'else'. > > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function") > Cc: Sinan Kaya <okaya@codeaurora.org> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Vinod Koul <vinod.koul@intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: new patch > drivers/acpi/bus.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index a87a97bf75f8..f3a7c29e9190 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -745,11 +745,13 @@ static const struct acpi_device_id *__acpi_match_device( > > 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 (ids) { > + for (id = ids; id->id[0] || id->cls; id++) { > + if (id->id[0] && !strcmp((char *)id->id, hwid->id)) > + return id; > + if (id->cls && __acpi_match_device_cls(id, hwid)) > + return id; > + } > } > > /* > Why not bail out here immediately if ids is null? __acpi_match_device() { /* * If the device is not present, it is unnecessary to load device * driver for it. */ if (!device || !device->status.present) return NULL; }
On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote: > On 2/1/2018 3:20 PM, Andy Shevchenko wrote: > > When __acpi_match_device() is called it would be possible to have > > ACPI ID table a MULL pointer. To avoid potential dereference, > > NULL Thanks, will fix later. > Why not bail out here immediately if ids is null? Because of the code which wasn't in context of this patch. See also patch 1 in the series. It's about how acpi_driver_match_device() uses it.
On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote: > On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote: > > On 2/1/2018 3:20 PM, Andy Shevchenko wrote: > > > When __acpi_match_device() is called it would be possible to have > > > ACPI ID table a MULL pointer. To avoid potential dereference, > > > > NULL > > Thanks, will fix later. > > > Why not bail out here immediately if ids is null? > > Because of the code which wasn't in context of this patch. > > See also patch 1 in the series. > > It's about how acpi_driver_match_device() uses it. Btw, it makes device_get_match_data() to work properly on PRP0001 kind of devices.
On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote: >> On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote: >> > On 2/1/2018 3:20 PM, Andy Shevchenko wrote: >> > > When __acpi_match_device() is called it would be possible to have >> > > ACPI ID table a MULL pointer. To avoid potential dereference, >> > >> > NULL >> >> Thanks, will fix later. >> >> > Why not bail out here immediately if ids is null? >> >> Because of the code which wasn't in context of this patch. >> >> See also patch 1 in the series. >> >> It's about how acpi_driver_match_device() uses it. > > Btw, it makes device_get_match_data() to work properly on PRP0001 kind > of devices. Maybe combine this one with the [1/6] then, it would make it somewhat easier to follow what's going on. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2018-02-04 at 08:18 +0100, Rafael J. Wysocki wrote: > On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote: > > > On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote: > > > > On 2/1/2018 3:20 PM, Andy Shevchenko wrote: > > > > > When __acpi_match_device() is called it would be possible to > > > > > have > > > > > ACPI ID table a MULL pointer. To avoid potential dereference, > > > > > > > > NULL > > > > > > Thanks, will fix later. > > > > > > > Why not bail out here immediately if ids is null? > > > > > > Because of the code which wasn't in context of this patch. > > > > > > See also patch 1 in the series. > > > > > > It's about how acpi_driver_match_device() uses it. > > > > Btw, it makes device_get_match_data() to work properly on PRP0001 > > kind > > of devices. > > Maybe combine this one with the [1/6] then, it would make it somewhat > easier to follow what's going on. Sure, I will fold in.
On Mon, 2018-02-05 at 17:56 +0200, Andy Shevchenko wrote: > On Sun, 2018-02-04 at 08:18 +0100, Rafael J. Wysocki wrote: > > On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko > > > > Maybe combine this one with the [1/6] then, it would make it > > somewhat > > easier to follow what's going on. > > Sure, I will fold in. Ah, wait, they have different Fixes tag. One goes deeply down in the history. So, I can instead put more explanations in patch 1. Would it work?
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index a87a97bf75f8..f3a7c29e9190 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -745,11 +745,13 @@ static const struct acpi_device_id *__acpi_match_device( 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 (ids) { + for (id = ids; id->id[0] || id->cls; id++) { + if (id->id[0] && !strcmp((char *)id->id, hwid->id)) + return id; + if (id->cls && __acpi_match_device_cls(id, hwid)) + return id; + } } /*
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. While here, remove redundant 'else'. Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function") Cc: Sinan Kaya <okaya@codeaurora.org> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v2: new patch drivers/acpi/bus.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)