All of lore.kernel.org
 help / color / mirror / Atom feed
* re: ACPI / scan: Simplify acpi_match_device()
@ 2015-04-13 19:21 Dan Carpenter
  2015-04-13 19:59 ` Rafael J. Wysocki
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2015-04-13 19:21 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-acpi

Hello Rafael J. Wysocki,

The patch e1acdeb0e770: "ACPI / scan: Simplify acpi_match_device()"
from Apr 10, 2015, leads to the following static checker warning:

	drivers/acpi/scan.c:269 acpi_companion_match()
	error: potential NULL dereference 'adev'.

drivers/acpi/scan.c
   247  static struct acpi_device *acpi_companion_match(const struct device *dev)
   248  {
   249          struct acpi_device *adev;
   250  
   251          adev = ACPI_COMPANION(dev);
   252          if (!adev)
   253                  return NULL;
   254  
   255          if (list_empty(&adev->pnp.ids))
   256                  return NULL;
   257  
   258          mutex_lock(&adev->physical_node_lock);
   259          if (list_empty(&adev->physical_node_list)) {
   260                  adev = NULL;
                        ^^^^^^^^^^^
   261          } else {
   262                  const struct acpi_device_physical_node *node;
   263  
   264                  node = list_first_entry(&adev->physical_node_list,
   265                                          struct acpi_device_physical_node, node);
   266                  if (node->dev != dev)
   267                          adev = NULL;
                                ^^^^^^^^^^^^
   268          }
   269          mutex_unlock(&adev->physical_node_lock);
                              ^^^^^^
Dereference.

   270  
   271          return adev;
   272  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ACPI / scan: Simplify acpi_match_device()
  2015-04-13 19:21 ACPI / scan: Simplify acpi_match_device() Dan Carpenter
@ 2015-04-13 19:59 ` Rafael J. Wysocki
  2015-04-14 10:37   ` Mika Westerberg
  0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2015-04-13 19:59 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: rafael.j.wysocki, linux-acpi, Mika Westerberg

On Monday, April 13, 2015 10:21:59 PM Dan Carpenter wrote:
> Hello Rafael J. Wysocki,
> 
> The patch e1acdeb0e770: "ACPI / scan: Simplify acpi_match_device()"
> from Apr 10, 2015, leads to the following static checker warning:
> 
> 	drivers/acpi/scan.c:269 acpi_companion_match()
> 	error: potential NULL dereference 'adev'.
> 
> drivers/acpi/scan.c
>    247  static struct acpi_device *acpi_companion_match(const struct device *dev)
>    248  {
>    249          struct acpi_device *adev;
>    250  
>    251          adev = ACPI_COMPANION(dev);
>    252          if (!adev)
>    253                  return NULL;
>    254  
>    255          if (list_empty(&adev->pnp.ids))
>    256                  return NULL;
>    257  
>    258          mutex_lock(&adev->physical_node_lock);
>    259          if (list_empty(&adev->physical_node_list)) {
>    260                  adev = NULL;
>                         ^^^^^^^^^^^
>    261          } else {
>    262                  const struct acpi_device_physical_node *node;
>    263  
>    264                  node = list_first_entry(&adev->physical_node_list,
>    265                                          struct acpi_device_physical_node, node);
>    266                  if (node->dev != dev)
>    267                          adev = NULL;
>                                 ^^^^^^^^^^^^
>    268          }
>    269          mutex_unlock(&adev->physical_node_lock);
>                               ^^^^^^
> Dereference.
> 
>    270  
>    271          return adev;
>    272  }

Right, thanks.

The patch below should fix it.

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI / scan: Fix NULL pointer dereference in acpi_companion_match()

Commit e1acdeb0e770 "ACPI / scan: Simplify acpi_match_device()"
introduced code that may lead to a NULL pointer dereference when
trying to unlock a mutex.  Fix that.

Fixes: e1acdeb0e770 "ACPI / scan: Simplify acpi_match_device()"
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -247,6 +247,7 @@ static int create_of_modalias(struct acp
 static struct acpi_device *acpi_companion_match(const struct device *dev)
 {
 	struct acpi_device *adev;
+	struct mutex *physical_node_lock;
 
 	adev = ACPI_COMPANION(dev);
 	if (!adev)
@@ -255,7 +256,8 @@ static struct acpi_device *acpi_companio
 	if (list_empty(&adev->pnp.ids))
 		return NULL;
 
-	mutex_lock(&adev->physical_node_lock);
+	physical_node_lock = &adev->physical_node_lock;
+	mutex_lock(physical_node_lock);
 	if (list_empty(&adev->physical_node_list)) {
 		adev = NULL;
 	} else {
@@ -266,7 +268,7 @@ static struct acpi_device *acpi_companio
 		if (node->dev != dev)
 			adev = NULL;
 	}
-	mutex_unlock(&adev->physical_node_lock);
+	mutex_unlock(physical_node_lock);
 
 	return adev;
 }


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ACPI / scan: Simplify acpi_match_device()
  2015-04-13 19:59 ` Rafael J. Wysocki
@ 2015-04-14 10:37   ` Mika Westerberg
  0 siblings, 0 replies; 3+ messages in thread
From: Mika Westerberg @ 2015-04-14 10:37 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Dan Carpenter, rafael.j.wysocki, linux-acpi

On Mon, Apr 13, 2015 at 09:59:48PM +0200, Rafael J. Wysocki wrote:
> On Monday, April 13, 2015 10:21:59 PM Dan Carpenter wrote:
> > Hello Rafael J. Wysocki,
> > 
> > The patch e1acdeb0e770: "ACPI / scan: Simplify acpi_match_device()"
> > from Apr 10, 2015, leads to the following static checker warning:
> > 
> > 	drivers/acpi/scan.c:269 acpi_companion_match()
> > 	error: potential NULL dereference 'adev'.
> > 
> > drivers/acpi/scan.c
> >    247  static struct acpi_device *acpi_companion_match(const struct device *dev)
> >    248  {
> >    249          struct acpi_device *adev;
> >    250  
> >    251          adev = ACPI_COMPANION(dev);
> >    252          if (!adev)
> >    253                  return NULL;
> >    254  
> >    255          if (list_empty(&adev->pnp.ids))
> >    256                  return NULL;
> >    257  
> >    258          mutex_lock(&adev->physical_node_lock);
> >    259          if (list_empty(&adev->physical_node_list)) {
> >    260                  adev = NULL;
> >                         ^^^^^^^^^^^
> >    261          } else {
> >    262                  const struct acpi_device_physical_node *node;
> >    263  
> >    264                  node = list_first_entry(&adev->physical_node_list,
> >    265                                          struct acpi_device_physical_node, node);
> >    266                  if (node->dev != dev)
> >    267                          adev = NULL;
> >                                 ^^^^^^^^^^^^
> >    268          }
> >    269          mutex_unlock(&adev->physical_node_lock);
> >                               ^^^^^^
> > Dereference.
> > 
> >    270  
> >    271          return adev;
> >    272  }
> 
> Right, thanks.
> 
> The patch below should fix it.
> 
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: ACPI / scan: Fix NULL pointer dereference in acpi_companion_match()
> 
> Commit e1acdeb0e770 "ACPI / scan: Simplify acpi_match_device()"
> introduced code that may lead to a NULL pointer dereference when
> trying to unlock a mutex.  Fix that.
> 
> Fixes: e1acdeb0e770 "ACPI / scan: Simplify acpi_match_device()"
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-04-14 10:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-13 19:21 ACPI / scan: Simplify acpi_match_device() Dan Carpenter
2015-04-13 19:59 ` Rafael J. Wysocki
2015-04-14 10:37   ` Mika Westerberg

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.