linux-kernel - [PATCH v1 1/1] ACPI: scan: Use unique number for instance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20210312160137.19463-1-andriy.shevchenko@linux.intel.com>
Date:   Fri, 12 Mar 2021 18:01:37 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: [PATCH v1 1/1] ACPI: scan: Use unique number for instance_no

Current mechanism of incrementing and decrementing plain integer
to get a next free instance_no when creating an ACPI device is fragile.

In case of hot plug event or namespace removal of the device instances
with the low numbers the plain integer counter can't cover the gaps
and become desynchronized with real state of affairs. If during next
hot plug event or namespace injection the new instances of
the devices need to be instantiated, the counter may mistakenly point
to the existing instance_no and kernel will complain:
"sysfs: cannot create duplicate filename '/bus/acpi/devices/XXXX1234:02'"

Replace plain integer approach by using IDA framework.

Fixes: e49bd2dd5a50 ("ACPI: use PNPID:instance_no as bus_id of ACPI device")
Fixes: ca9dc8d42b30 ("ACPI / scan: Fix acpi_bus_id_list bookkeeping")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
---
 drivers/acpi/internal.h |  4 ++-
 drivers/acpi/scan.c     | 55 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index e6a5d997241c..6fee4f71ba1c 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -9,6 +9,8 @@
 #ifndef _ACPI_INTERNAL_H_
 #define _ACPI_INTERNAL_H_
 
+#include <linux/idr.h>
+
 #define PREFIX "ACPI: "
 
 int early_acpi_osi_init(void);
@@ -98,7 +100,7 @@ extern struct list_head acpi_bus_id_list;
 
 struct acpi_device_bus_id {
 	const char *bus_id;
-	unsigned int instance_no;
+	struct ida no;
 	struct list_head node;
 };
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a184529d8fa4..a118a58f7dad 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -468,9 +468,27 @@ static void acpi_device_release(struct device *dev)
 	kfree(acpi_dev);
 }
 
+static int acpi_device_get_instance_no(struct acpi_device *device)
+{
+	const char *p;
+	int result;
+	int error;
+
+	p = strrchr(dev_name(&device->dev), ':');
+	if (!p)
+		return -ENODATA;
+
+	error = kstrtoint(p + 1, 16, &result);
+	if (error)
+		return error;
+
+	return result;
+}
+
 static void acpi_device_del(struct acpi_device *device)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
+	int result;
 
 	mutex_lock(&acpi_device_lock);
 	if (device->parent)
@@ -479,9 +497,13 @@ static void acpi_device_del(struct acpi_device *device)
 	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node)
 		if (!strcmp(acpi_device_bus_id->bus_id,
 			    acpi_device_hid(device))) {
-			if (acpi_device_bus_id->instance_no > 0)
-				acpi_device_bus_id->instance_no--;
-			else {
+			result = acpi_device_get_instance_no(device);
+			if (result < 0)
+				dev_warn(&device->dev, "Can't get instance no\n");
+			else
+				ida_simple_remove(&acpi_device_bus_id->no, result);
+
+			if (ida_is_empty(&acpi_device_bus_id->no)) {
 				list_del(&acpi_device_bus_id->node);
 				kfree_const(acpi_device_bus_id->bus_id);
 				kfree(acpi_device_bus_id);
@@ -631,6 +653,19 @@ static struct acpi_device_bus_id *acpi_device_bus_id_match(const char *dev_id)
 	return NULL;
 }
 
+static int acpi_device_set_name(struct acpi_device *device,
+				struct acpi_device_bus_id *acpi_device_bus_id)
+{
+	int result;
+
+	result = ida_simple_get(&acpi_device_bus_id->no, 0, 255, GFP_KERNEL);
+	if (result < 0)
+		return result;
+
+	dev_set_name(&device->dev, "%s:%02x", acpi_device_bus_id->bus_id, result);
+	return 0;
+}
+
 int acpi_device_add(struct acpi_device *device,
 		    void (*release)(struct device *))
 {
@@ -665,7 +700,9 @@ int acpi_device_add(struct acpi_device *device,
 
 	acpi_device_bus_id = acpi_device_bus_id_match(acpi_device_hid(device));
 	if (acpi_device_bus_id) {
-		acpi_device_bus_id->instance_no++;
+		result = acpi_device_set_name(device, acpi_device_bus_id);
+		if (result < 0)
+			goto err_unlock;
 	} else {
 		acpi_device_bus_id = kzalloc(sizeof(*acpi_device_bus_id),
 					     GFP_KERNEL);
@@ -681,9 +718,17 @@ int acpi_device_add(struct acpi_device *device,
 			goto err_unlock;
 		}
 
+		ida_init(&acpi_device_bus_id->no);
+
+		result = acpi_device_set_name(device, acpi_device_bus_id);
+		if (result < 0) {
+			ida_destroy(&acpi_device_bus_id->no);
+			kfree(acpi_device_bus_id);
+			goto err_unlock;
+		}
+
 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
 	}
-	dev_set_name(&device->dev, "%s:%02x", acpi_device_bus_id->bus_id, acpi_device_bus_id->instance_no);
 
 	if (device->parent)
 		list_add_tail(&device->node, &device->parent->children);
-- 
2.30.1