All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: dmaengine <dmaengine@vger.kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Sinan Kaya <okaya@codeaurora.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Vinod Koul <vinod.koul@intel.com>
Subject: [v3,2/5] ACPI / bus: Do not traverse through non-existed device table
Date: Thu, 08 Feb 2018 18:53:22 +0200	[thread overview]
Message-ID: <1518108802.22495.207.camel@linux.intel.com> (raw)

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@linux.intel.com> 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@linux.intel.com>
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@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>
---
 drivers/acpi/bus.c | 66 +++++++++++++++++++++++++++++++++------------
---------
 1 file changed, 41 insertions(+), 25 deletions(-)

+				 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);

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)

WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: dmaengine <dmaengine@vger.kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Sinan Kaya <okaya@codeaurora.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Vinod Koul <vinod.koul@intel.com>
Subject: Re: [PATCH v3 2/5] ACPI / bus: Do not traverse through non-existed device table
Date: Thu, 08 Feb 2018 18:53:22 +0200	[thread overview]
Message-ID: <1518108802.22495.207.camel@linux.intel.com> (raw)
In-Reply-To: <CAJZ5v0iEB74dPUN400pfB4+AV--NZj7hs6aZGiSoSiz5cTS37g@mail.gmail.com>

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@linux.intel.com> 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@linux.intel.com>
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@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>
---
 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@linux.intel.com>
Intel Finland Oy

             reply	other threads:[~2018-02-08 16:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08 16:53 Andy Shevchenko [this message]
2018-02-08 16:53 ` [PATCH v3 2/5] ACPI / bus: Do not traverse through non-existed device table Andy Shevchenko
  -- strict thread matches above, loose matches on Subject: below --
2018-02-08 17:06 [v3,2/5] " Rafael J. Wysocki
2018-02-08 17:06 ` [PATCH v3 2/5] " Rafael J. Wysocki
2018-02-08 16:13 [v3,2/5] " Andy Shevchenko
2018-02-08 16:13 ` [PATCH v3 2/5] " Andy Shevchenko
2018-02-08 16:05 [v3,1/5] ACPI / bus: Return error code from __acpi_match_device() in one case Rafael J. Wysocki
2018-02-08 16:05 ` [PATCH v3 1/5] " Rafael J. Wysocki
2018-02-08 16:01 [v3,2/5] ACPI / bus: Do not traverse through non-existed device table Rafael J. Wysocki
2018-02-08 16:01 ` [PATCH v3 2/5] " Rafael J. Wysocki
2018-02-08 15:59 [v3,2/5] " Rafael J. Wysocki
2018-02-08 15:59 ` [PATCH v3 2/5] " Rafael J. Wysocki
2018-02-08 15:59 [v3,1/5] ACPI / bus: Return error code from __acpi_match_device() in one case Andy Shevchenko
2018-02-08 15:59 ` [PATCH v3 1/5] " Andy Shevchenko
2018-02-08 15:48 [v3,1/5] " Rafael J. Wysocki
2018-02-08 15:48 ` [PATCH v3 1/5] " Rafael J. Wysocki
2018-02-08 15:45 [v3,1/5] " Rafael J. Wysocki
2018-02-08 15:45 ` [PATCH v3 1/5] " Rafael J. Wysocki
2018-02-08 15:44 [v3,1/5] " Andy Shevchenko
2018-02-08 15:44 ` [PATCH v3 1/5] " Andy Shevchenko
2018-02-08 15:14 [v3,1/5] " Rafael J. Wysocki
2018-02-08 15:14 ` [PATCH v3 1/5] " Rafael J. Wysocki
2018-02-07 14:56 [v3,5/5] device property: Constify device_get_match_data() Andy Shevchenko
2018-02-07 14:56 ` [PATCH v3 5/5] " Andy Shevchenko
2018-02-07 14:56 [v3,4/5] ACPI / bus: Rename acpi_get_match_data() to acpi_device_get_match_data() Andy Shevchenko
2018-02-07 14:56 ` [PATCH v3 4/5] " Andy Shevchenko
2018-02-07 14:56 [v3,3/5] ACPI / bus: Remove checks in acpi_get_match_data() Andy Shevchenko
2018-02-07 14:56 ` [PATCH v3 3/5] " Andy Shevchenko
2018-02-07 14:56 [v3,2/5] ACPI / bus: Do not traverse through non-existed device table Andy Shevchenko
2018-02-07 14:56 ` [PATCH v3 2/5] " Andy Shevchenko
2018-02-07 14:56 [v3,1/5] ACPI / bus: Return error code from __acpi_match_device() in one case Andy Shevchenko
2018-02-07 14:56 ` [PATCH v3 1/5] " Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1518108802.22495.207.camel@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=okaya@codeaurora.org \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sakari.ailus@linux.intel.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.