[PATCH v1 2/4] ACPI: glue: Rearrange acpi_bind_one() to avoid breakage

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>

Rearange the code in acpi_bind_one() to avoid situations in which the
existing ACPI companion of the given device would be replaced with NULL
due to a memory allocation error or because somebody tries to bind a
physical device with an ACPI companion to a different ACPI device
erroneously.

Fixes: 7b1998116bbb ("ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
 drivers/acpi/glue.c | 59 +++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 40e6513a9942..89336a5fa78b 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -243,31 +243,26 @@ static void acpi_physnode_link_name(char *buf, unsigned int node_id)
 int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 {
 	struct acpi_device_physical_node *physical_node, *pn;
+	struct acpi_device *comp_dev = ACPI_COMPANION(dev);
 	char physical_node_name[PHYSICAL_NODE_NAME_SIZE];
 	struct list_head *physnode_list;
 	unsigned int node_id;
 	int retval = -EINVAL;
 
-	if (has_acpi_companion(dev)) {
-		if (acpi_dev) {
-			dev_warn(dev, "ACPI companion already set\n");
+	if (!acpi_dev) {
+		if (!comp_dev)
 			return -EINVAL;
-		} else {
-			acpi_dev = ACPI_COMPANION(dev);
-		}
-	}
-	if (!acpi_dev)
-		return -EINVAL;
 
-	acpi_dev_get(acpi_dev);
-	get_device(dev);
-	physical_node = kzalloc_obj(*physical_node);
-	if (!physical_node) {
-		retval = -ENOMEM;
-		goto err;
+		/* If the companion has been set upfront, pick it up. */
+		acpi_dev = comp_dev;
+	}
+	if (comp_dev && comp_dev != acpi_dev) {
+		dev_warn(dev, "ACPI companion already set to %s which is not %s\n",
+			 acpi_dev_name(comp_dev), acpi_dev_name(acpi_dev));
+		return -EEXIST;
 	}
 
-	mutex_lock(&acpi_dev->physical_node_lock);
+	guard(mutex)(&acpi_dev->physical_node_lock);
 
 	/*
 	 * Keep the list sorted by node_id so that the IDs of removed nodes can
@@ -278,15 +273,12 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	list_for_each_entry(pn, &acpi_dev->physical_node_list, node) {
 		/* Sanity check. */
 		if (pn->dev == dev) {
-			mutex_unlock(&acpi_dev->physical_node_lock);
-
-			dev_warn(dev, "Already associated with ACPI node\n");
-			kfree(physical_node);
-			if (ACPI_COMPANION(dev) != acpi_dev)
-				goto err;
-
-			put_device(dev);
-			acpi_dev_put(acpi_dev);
+			if (!comp_dev) {
+				/* Really unexpected. */
+				ACPI_COMPANION_SET(dev, acpi_dev);
+				dev_warn(&acpi_dev->dev,
+					 "Physical device list corruption fixed up\n");
+			}
 			return 0;
 		}
 		if (pn->node_id == node_id) {
@@ -295,12 +287,19 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 		}
 	}
 
+	physical_node = kzalloc_obj(*physical_node);
+	if (!physical_node)
+		return -ENOMEM;
+
+	acpi_dev_get(acpi_dev);
+	get_device(dev);
+
 	physical_node->node_id = node_id;
 	physical_node->dev = dev;
 	list_add(&physical_node->node, physnode_list);
 	acpi_dev->physical_node_count++;
 
-	if (!has_acpi_companion(dev))
+	if (!comp_dev)
 		ACPI_COMPANION_SET(dev, acpi_dev);
 
 	acpi_physnode_link_name(physical_node_name, node_id);
@@ -316,18 +315,10 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 		dev_err(dev, "Failed to create link firmware_node (%d)\n",
 			retval);
 
-	mutex_unlock(&acpi_dev->physical_node_lock);
-
 	if (acpi_dev->wakeup.flags.valid)
 		device_set_wakeup_capable(dev, true);
 
 	return 0;
-
- err:
-	ACPI_COMPANION_SET(dev, NULL);
-	put_device(dev);
-	acpi_dev_put(acpi_dev);
-	return retval;
 }
 EXPORT_SYMBOL_GPL(acpi_bind_one);
 
-- 
2.51.0