[PATCH] ACPI: sysfs: Serialize access to the table attribute list

[PATCH] ACPI: sysfs: Serialize access to the table attribute list

[Jamie Nguyen]

acpi_table_attr_init() walks acpi_table_attr_list to pick the instance
number for a table's sysfs file, and its callers then add the new entry
to the list, but nothing serializes this. ACPICA drops its table lock
before calling the table install handler, so tables installed at the
same time can pick the same instance number:

  sysfs: cannot create duplicate filename '/firmware/acpi/tables/dynamic/SSDT1'

When this happens, the table is installed, but has no file under
dynamic/. Concurrent list_add_tail() calls can also corrupt the list.

Add a mutex around the instance lookup, the file creation and the list
addition, and move the list addition into acpi_table_attr_init() so it
is done under the mutex.

Fixes: 7a37052adb5e ("ACPICA: Tables: Fix hidden logic related to acpi_tb_install_standard_table()")
Signed-off-by: Jamie Nguyen <jamien@nvidia.com>
---
Sashiko flagged this, as a pre-existing issue, in its review of my
previous patch:
https://sashiko.dev/#/patchset/20261110225452.2349540-1-jamien%40nvidia.com

Tested on an arm64 server (288 CPUs) and in QEMU (x86_64, 8 CPUs) by
loading SSDTs through ACPI configfs from 8 to 64 processes at once:
- Without this patch, 3 to 7 tables per run got a duplicate file name
  and had no file under dynamic/.
- With it, every table got its own file, with no lockdep or list
  debugging warnings.
- The boot table files are unchanged.

 drivers/acpi/sysfs.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index d1b57d1246aafb1..47b816c588feedd 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/kstrtox.h>
 #include <linux/moduleparam.h>
+#include <linux/mutex.h>
 #include <linux/string.h>
 
 #include "internal.h"
@@ -298,6 +299,7 @@ module_param_call(acpica_version, NULL, param_get_acpica_version, NULL, 0444);
  */
 
 static LIST_HEAD(acpi_table_attr_list);
+static DEFINE_MUTEX(acpi_table_attr_lock);
 static struct kobject *tables_kobj;
 static struct kobject *tables_data_kobj;
 static struct kobject *dynamic_tables_kobj;
@@ -348,6 +350,9 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
 	struct acpi_table_header *header = NULL;
 	struct acpi_table_attr *attr = NULL;
 	char instance_str[ACPI_INST_SIZE];
+	int ret;
+
+	guard(mutex)(&acpi_table_attr_lock);
 
 	sysfs_attr_init(&table_attr->attr.attr);
 	ACPI_COPY_NAMESEG(table_attr->name, table_header->signature);
@@ -379,7 +384,12 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
 	table_attr->attr.attr.name = table_attr->filename;
 	table_attr->attr.attr.mode = 0400;
 
-	return sysfs_create_bin_file(tables_obj, &table_attr->attr);
+	ret = sysfs_create_bin_file(tables_obj, &table_attr->attr);
+	if (ret)
+		return ret;
+
+	list_add_tail(&table_attr->node, &acpi_table_attr_list);
+	return 0;
 }
 
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
@@ -397,7 +407,6 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 			kfree(table_attr);
 			return AE_ERROR;
 		}
-		list_add_tail(&table_attr->node, &acpi_table_attr_list);
 		break;
 	case ACPI_TABLE_EVENT_LOAD:
 	case ACPI_TABLE_EVENT_UNLOAD:
@@ -604,7 +613,6 @@ static int acpi_tables_sysfs_init(void)
 			acpi_put_table(table_header);
 			break;
 		}
-		list_add_tail(&table_attr->node, &acpi_table_attr_list);
 		acpi_table_data_init(table_header, table_attr);
 		acpi_put_table(table_header);
 	}

base-commit: 03176329b3f11ca2298f310a47486a5bd2f13d1d
-- 
2.43.0