Skip to contentSkip to conversation

ACPI / scan: Simplify acpi_match_device()

DISCUSSION Dan Carpenter 3 participants 3 messages Apr 13, 2015 – Apr 14, 2015 ACPI

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 35 lines · Dan Carpenter

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);
                              ^^^^^^
Quoted 5 lines

Dereference.

   270  
   271          return adev;
   272  }

Right, thanks.

The patch below should fix it.

---
From: Rafael J. Wysocki <[email protected]>
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()"
---
 drivers/acpi/scan.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/acpi/scan.c +4 -2
Index: 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;
 }
Reviewed-by
#2 lore raw
Quoted 50 lines · Rafael J. Wysocki
Quoted 35 lines · Dan Carpenter

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);
                              ^^^^^^
Quoted 5 lines

Dereference.

   270  
   271          return adev;
   272  }

Right, thanks.

The patch below should fix it.

---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / scan: Fix NULL pointer dereference in acpi_companion_match()

Quoted 6 lines

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()"