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: <20250723121034.3685996-1-lihuisong@huawei.com>
Date: Wed, 23 Jul 2025 20:10:34 +0800
From: Huisong Li <lihuisong@...wei.com>
To: <rafael@...nel.org>, <lenb@...nel.org>
CC: <linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linuxarm@...wei.com>, <jonathan.cameron@...wei.com>,
	<zhanjie9@...ilicon.com>, <zhenglifeng1@...wei.com>, <yubowen8@...wei.com>,
	<liuyonglong@...wei.com>, <lihuisong@...wei.com>
Subject: [PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

There are three kind of issues:
1> There are two resource leak issues in acpi_processor_power_init:
   a> Don't unregister acpi_idle_driver when do kzalloc failed.
   b> Don't free cpuidle device memory when register cpuidle device failed.
2> There isn't lock to prevent the global acpi_processor_registered, which
   may lead to concurrent register cpuidle driver.
3> The cpuidle driver should be registered in advance when all of the CPUs
   have been brought up instead of being in a CPU hotplug callback.

To solve these issues, so add a new function to initialize acpi_idle_driver
based on the power management information of an available CPU and register
cpuidle driver in acpi_processor_driver_init().

Signed-off-by: Huisong Li <lihuisong@...wei.com>
---
 v2: register cpuidle driver in advance when all of the CPUs have been
     brought up.
     v1 link: https://patchwork.kernel.org/project/linux-acpi/patch/20250619061327.1674384-1-lihuisong@huawei.com/
---
 drivers/acpi/processor_driver.c |  5 +++
 drivers/acpi/processor_idle.c   | 71 ++++++++++++++++++++++-----------
 include/acpi/processor.h        |  9 +++++
 3 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 65e779be64ff..ff944c93b6ff 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -263,6 +263,10 @@ static int __init acpi_processor_driver_init(void)
 	if (result < 0)
 		return result;
 
+	result = acpi_processor_register_idle_driver();
+	if (result)
+		pr_info("register idle driver failed, ret = %d.\n", result);
+
 	result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
 				   "acpi/cpu-drv:online",
 				   acpi_soft_cpu_online, NULL);
@@ -301,6 +305,7 @@ static void __exit acpi_processor_driver_exit(void)
 
 	cpuhp_remove_state_nocalls(hp_online);
 	cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
+	acpi_processor_unregister_idle_driver();
 	driver_unregister(&acpi_processor_driver);
 }
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2c2dc559e0f8..2408f1076631 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1360,7 +1360,52 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
 	return 0;
 }
 
-static int acpi_processor_registered;
+int acpi_processor_register_idle_driver(void)
+{
+	struct acpi_processor *pr;
+	int cpu;
+	int ret;
+
+	/*
+	 * Acpi idle driver is used by all possible CPUs.
+	 * Install the idle handler by the processor power info of one in them.
+	 * Note that we use previously set idle handler will be used on
+	 * platforms that only support C1.
+	 */
+	for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
+		pr = per_cpu(processors, cpu);
+		if (pr == NULL)
+			continue;
+
+		ret = acpi_processor_get_power_info(pr);
+		if (!ret) {
+			pr->flags.power_setup_done = 1;
+			break;
+		}
+	}
+
+	if (unlikely(!pr))
+		return -ENODEV;
+
+	if (ret) {
+		pr_err("%s get power information failed.\n",
+		       acpi_idle_driver.name);
+		return ret;
+	}
+
+	acpi_processor_setup_cpuidle_states(pr);
+	ret = cpuidle_register_driver(&acpi_idle_driver);
+	if (ret)
+		return ret;
+
+	pr_debug("%s registered with cpuidle\n", acpi_idle_driver.name);
+	return 0;
+}
+
+void acpi_processor_unregister_idle_driver(void)
+{
+	cpuidle_unregister_driver(&acpi_idle_driver);
+}
 
 int acpi_processor_power_init(struct acpi_processor *pr)
 {
@@ -1375,22 +1420,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 	if (!acpi_processor_get_power_info(pr))
 		pr->flags.power_setup_done = 1;
 
-	/*
-	 * Install the idle handler if processor power management is supported.
-	 * Note that we use previously set idle handler will be used on
-	 * platforms that only support C1.
-	 */
 	if (pr->flags.power) {
-		/* Register acpi_idle_driver if not already registered */
-		if (!acpi_processor_registered) {
-			acpi_processor_setup_cpuidle_states(pr);
-			retval = cpuidle_register_driver(&acpi_idle_driver);
-			if (retval)
-				return retval;
-			pr_debug("%s registered with cpuidle\n",
-				 acpi_idle_driver.name);
-		}
-
 		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 		if (!dev)
 			return -ENOMEM;
@@ -1403,11 +1433,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 		 */
 		retval = cpuidle_register_device(dev);
 		if (retval) {
-			if (acpi_processor_registered == 0)
-				cpuidle_unregister_driver(&acpi_idle_driver);
+			per_cpu(acpi_cpuidle_device, pr->id) = NULL;
+			kfree(dev);
 			return retval;
 		}
-		acpi_processor_registered++;
 	}
 	return 0;
 }
@@ -1421,10 +1450,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
 
 	if (pr->flags.power) {
 		cpuidle_unregister_device(dev);
-		acpi_processor_registered--;
-		if (acpi_processor_registered == 0)
-			cpuidle_unregister_driver(&acpi_idle_driver);
-
 		kfree(dev);
 	}
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index d0eccbd920e5..3cb41a3f2d9a 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
 int acpi_processor_power_exit(struct acpi_processor *pr);
 int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
 int acpi_processor_hotplug(struct acpi_processor *pr);
+int acpi_processor_register_idle_driver(void);
+void acpi_processor_unregister_idle_driver(void);
 #else
 static inline int acpi_processor_power_init(struct acpi_processor *pr)
 {
@@ -443,6 +445,13 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
 {
 	return -ENODEV;
 }
+static int acpi_processor_register_idle_driver(void)
+{
+	return -ENODEV;
+}
+static void acpi_processor_unregister_idle_driver(void)
+{
+}
 #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
 
 /* in processor_thermal.c */
-- 
2.33.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ