mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] platform/x86: asus-armoury: fix init errors and modernize resource management
@ 2026-07-03 10:31 Marco Scardovi
  2026-07-03 10:31 ` [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init Marco Scardovi
  2026-07-03 10:31 ` [PATCH v3 2/2] platform/x86: asus-armoury: use cleanup.h to manage tunables Marco Scardovi
  0 siblings, 2 replies; 4+ messages in thread
From: Marco Scardovi @ 2026-07-03 10:31 UTC (permalink / raw)
  To: ilpo.jarvinen
  Cc: corentin.chary, denis.benato, hansg, linux-kernel, luke,
	platform-driver-x86, Marco Scardovi

This patch series addresses a Use-After-Free (UAF) vulnerability
and a memory leak during driver initialization in the asus-armoury
driver, and modernizes its memory management.

Patch 1 fixes the UAF and memory leak by propagating the error code
from init_rog_tunables(), deferring the global pointer updates, and
adding a rollback path.

Patch 2 refactors init_rog_tunables() to use the cleanup infrastructure
via __free(kfree) and no_free_ptr() to automate memory management and
prevent future leaks.

Changes in v3:
- Added a second patch to convert init_rog_tunables() to use the
  __free(kfree) and no_free_ptr() cleanup helper macros.
- Added #include <linux/cleanup.h>.

Changes in v2:
- Restructured init_rog_tunables() to use local pointers
  (ac_rog_tunables, dc_rog_tunables) and only update the global
  static asus_armoury.rog_tunables structure once all allocations
  have successfully succeeded. This removes the risk of exposing
  freed or partially allocated pointers to the global struct.
- Removed redundant assignments to NULL in the error paths. Since
  initialization fails and the driver is not loaded, cleaning the
  global static pointers to NULL is not necessary.
- In init_rog_tunables(), replaced 'goto err_nomem' with direct
  'return -ENOMEM'. The 'err_nomem' label in the original code
  printed a generic OOM error message, which is discouraged in the
  kernel as the allocator already issues OOM details. Because that
  label did not perform rollback cleanup, a direct return is cleaner
  and simpler.
- Replaced inline cleanup in asus_fw_init() with a standard goto
  rollback block at the end of the function for cleaner error path
  styling.
Thread for v2:
https://lore.kernel.org/platform-driver-x86/20260701164333.5219-1-scardracs@disroot.org/

Marco Scardovi (2):
  platform/x86: asus-armoury: fix Use-After-Free and memory leak in
    driver init
  platform/x86: asus-armoury: use cleanup.h to manage tunables

 drivers/platform/x86/asus-armoury.c | 46 ++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 18 deletions(-)

-- 
2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init
  2026-07-03 10:31 [PATCH v3 0/2] platform/x86: asus-armoury: fix init errors and modernize resource management Marco Scardovi
@ 2026-07-03 10:31 ` Marco Scardovi
  2026-07-07 16:54   ` Ilpo Järvinen
  2026-07-03 10:31 ` [PATCH v3 2/2] platform/x86: asus-armoury: use cleanup.h to manage tunables Marco Scardovi
  1 sibling, 1 reply; 4+ messages in thread
From: Marco Scardovi @ 2026-07-03 10:31 UTC (permalink / raw)
  To: ilpo.jarvinen
  Cc: corentin.chary, denis.benato, hansg, linux-kernel, luke,
	platform-driver-x86, Marco Scardovi

In init_rog_tunables(), if dc_limits are defined and allocating
dc_rog_tunables fails, the already allocated ac_rog_tunables gets
freed but the pointer stored in
asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] is not cleared. Since
init_rog_tunables() returns void, the driver initialization continues,
which can lead to a Use-After-Free (UAF) when asus_fw_attr_add()
accesses the freed AC tunables pointer.

Additionally, if init_rog_tunables() succeeds but asus_fw_attr_add()
fails, the allocated tunables are not freed, resulting in a memory leak.

Fix these issues by making init_rog_tunables() return an error code and
propagating it in asus_fw_init(). Defer setting the global pointers in
asus_armoury.rog_tunables until both tunables have been successfully
allocated. If asus_fw_attr_add() fails, release the allocated resources
using a standard goto rollback block in asus_fw_init().

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/platform/x86/asus-armoury.c | 38 ++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
index 495dc1e31d40..ea53a005bb25 100644
--- a/drivers/platform/x86/asus-armoury.c
+++ b/drivers/platform/x86/asus-armoury.c
@@ -989,7 +989,7 @@ static int asus_fw_attr_add(void)
 /* Init / exit ****************************************************************/
 
 /* Set up the min/max and defaults for ROG tunables */
-static void init_rog_tunables(void)
+static int init_rog_tunables(void)
 {
 	const struct power_limits *ac_limits, *dc_limits;
 	struct rog_tunables *ac_rog_tunables = NULL, *dc_rog_tunables = NULL;
@@ -1000,24 +1000,23 @@ static void init_rog_tunables(void)
 	dmi_id = dmi_first_match(power_limits);
 	if (!dmi_id) {
 		pr_warn("No matching power limits found for this system\n");
-		return;
+		return 0;
 	}
 
 	/* Get the power data for this system */
 	power_data = dmi_id->driver_data;
 	if (!power_data) {
 		pr_info("No power data available for this system\n");
-		return;
+		return 0;
 	}
 
 	/* Initialize AC power tunables */
 	ac_limits = power_data->ac_data;
 	if (ac_limits) {
-		ac_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
+		ac_rog_tunables = kzalloc_obj(*ac_rog_tunables);
 		if (!ac_rog_tunables)
-			goto err_nomem;
+			return -ENOMEM;
 
-		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = ac_rog_tunables;
 		ac_rog_tunables->power_limits = ac_limits;
 
 		/* Set initial AC values */
@@ -1060,13 +1059,12 @@ static void init_rog_tunables(void)
 	/* Initialize DC power tunables */
 	dc_limits = power_data->dc_data;
 	if (dc_limits) {
-		dc_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
+		dc_rog_tunables = kzalloc_obj(*dc_rog_tunables);
 		if (!dc_rog_tunables) {
 			kfree(ac_rog_tunables);
-			goto err_nomem;
+			return -ENOMEM;
 		}
 
-		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
 		dc_rog_tunables->power_limits = dc_limits;
 
 		/* Set initial DC values */
@@ -1106,15 +1104,16 @@ static void init_rog_tunables(void)
 		pr_debug("No DC PPT limits defined\n");
 	}
 
-	return;
+	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = ac_rog_tunables;
+	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
 
-err_nomem:
-	pr_err("Failed to allocate memory for tunables\n");
+	return 0;
 }
 
 static int __init asus_fw_init(void)
 {
 	char *wmi_uid;
+	int err;
 
 	wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
 	if (!wmi_uid)
@@ -1127,10 +1126,21 @@ static int __init asus_fw_init(void)
 	if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
 		return -ENODEV;
 
-	init_rog_tunables();
+	err = init_rog_tunables();
+	if (err)
+		return err;
 
 	/* Must always be last step to ensure data is available */
-	return asus_fw_attr_add();
+	err = asus_fw_attr_add();
+	if (err)
+		goto err_free_tunables;
+
+	return 0;
+
+err_free_tunables:
+	kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
+	kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
+	return err;
 }
 
 static void __exit asus_fw_exit(void)
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] platform/x86: asus-armoury: use cleanup.h to manage tunables
  2026-07-03 10:31 [PATCH v3 0/2] platform/x86: asus-armoury: fix init errors and modernize resource management Marco Scardovi
  2026-07-03 10:31 ` [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init Marco Scardovi
@ 2026-07-03 10:31 ` Marco Scardovi
  1 sibling, 0 replies; 4+ messages in thread
From: Marco Scardovi @ 2026-07-03 10:31 UTC (permalink / raw)
  To: ilpo.jarvinen
  Cc: corentin.chary, denis.benato, hansg, linux-kernel, luke,
	platform-driver-x86, Marco Scardovi

Convert init_rog_tunables() to use the cleanup infrastructure to
automatically free ac_rog_tunables if dc_rog_tunables allocation fails.

By declaring local pointers with the __free(kfree) attribute, the
manual kfree() in the error path can be removed. Upon successful
initialization, the ownership is transferred to the global struct
using no_free_ptr().

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/platform/x86/asus-armoury.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
index ea53a005bb25..5af4a79a61b4 100644
--- a/drivers/platform/x86/asus-armoury.c
+++ b/drivers/platform/x86/asus-armoury.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
@@ -992,7 +993,8 @@ static int asus_fw_attr_add(void)
 static int init_rog_tunables(void)
 {
 	const struct power_limits *ac_limits, *dc_limits;
-	struct rog_tunables *ac_rog_tunables = NULL, *dc_rog_tunables = NULL;
+	struct rog_tunables *ac_rog_tunables __free(kfree) = NULL;
+	struct rog_tunables *dc_rog_tunables __free(kfree) = NULL;
 	const struct power_data *power_data;
 	const struct dmi_system_id *dmi_id;
 
@@ -1060,10 +1062,8 @@ static int init_rog_tunables(void)
 	dc_limits = power_data->dc_data;
 	if (dc_limits) {
 		dc_rog_tunables = kzalloc_obj(*dc_rog_tunables);
-		if (!dc_rog_tunables) {
-			kfree(ac_rog_tunables);
+		if (!dc_rog_tunables)
 			return -ENOMEM;
-		}
 
 		dc_rog_tunables->power_limits = dc_limits;
 
@@ -1104,8 +1104,8 @@ static int init_rog_tunables(void)
 		pr_debug("No DC PPT limits defined\n");
 	}
 
-	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = ac_rog_tunables;
-	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
+	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = no_free_ptr(ac_rog_tunables);
+	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = no_free_ptr(dc_rog_tunables);
 
 	return 0;
 }
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init
  2026-07-03 10:31 ` [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init Marco Scardovi
@ 2026-07-07 16:54   ` Ilpo Järvinen
  0 siblings, 0 replies; 4+ messages in thread
From: Ilpo Järvinen @ 2026-07-07 16:54 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: corentin.chary, denis.benato, Hans de Goede, LKML, luke,
	platform-driver-x86

On Fri, 3 Jul 2026, Marco Scardovi wrote:

> In init_rog_tunables(), if dc_limits are defined and allocating
> dc_rog_tunables fails, the already allocated ac_rog_tunables gets
> freed but the pointer stored in
> asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] is not cleared. Since
> init_rog_tunables() returns void, the driver initialization continues,
> which can lead to a Use-After-Free (UAF) when asus_fw_attr_add()
> accesses the freed AC tunables pointer.
> 
> Additionally, if init_rog_tunables() succeeds but asus_fw_attr_add()
> fails, the allocated tunables are not freed, resulting in a memory leak.
> 
> Fix these issues by making init_rog_tunables() return an error code and
> propagating it in asus_fw_init(). Defer setting the global pointers in
> asus_armoury.rog_tunables until both tunables have been successfully
> allocated. If asus_fw_attr_add() fails, release the allocated resources
> using a standard goto rollback block in asus_fw_init().
> 
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>

This patch no longer applies cleanly, please rebase and send v4.

--
 i.

> ---
>  drivers/platform/x86/asus-armoury.c | 38 ++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 495dc1e31d40..ea53a005bb25 100644
> --- a/drivers/platform/x86/asus-armoury.c
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -989,7 +989,7 @@ static int asus_fw_attr_add(void)
>  /* Init / exit ****************************************************************/
>  
>  /* Set up the min/max and defaults for ROG tunables */
> -static void init_rog_tunables(void)
> +static int init_rog_tunables(void)
>  {
>  	const struct power_limits *ac_limits, *dc_limits;
>  	struct rog_tunables *ac_rog_tunables = NULL, *dc_rog_tunables = NULL;
> @@ -1000,24 +1000,23 @@ static void init_rog_tunables(void)
>  	dmi_id = dmi_first_match(power_limits);
>  	if (!dmi_id) {
>  		pr_warn("No matching power limits found for this system\n");
> -		return;
> +		return 0;
>  	}
>  
>  	/* Get the power data for this system */
>  	power_data = dmi_id->driver_data;
>  	if (!power_data) {
>  		pr_info("No power data available for this system\n");
> -		return;
> +		return 0;
>  	}
>  
>  	/* Initialize AC power tunables */
>  	ac_limits = power_data->ac_data;
>  	if (ac_limits) {
> -		ac_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
> +		ac_rog_tunables = kzalloc_obj(*ac_rog_tunables);
>  		if (!ac_rog_tunables)
> -			goto err_nomem;
> +			return -ENOMEM;
>  
> -		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = ac_rog_tunables;
>  		ac_rog_tunables->power_limits = ac_limits;
>  
>  		/* Set initial AC values */
> @@ -1060,13 +1059,12 @@ static void init_rog_tunables(void)
>  	/* Initialize DC power tunables */
>  	dc_limits = power_data->dc_data;
>  	if (dc_limits) {
> -		dc_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
> +		dc_rog_tunables = kzalloc_obj(*dc_rog_tunables);
>  		if (!dc_rog_tunables) {
>  			kfree(ac_rog_tunables);
> -			goto err_nomem;
> +			return -ENOMEM;
>  		}
>  
> -		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
>  		dc_rog_tunables->power_limits = dc_limits;
>  
>  		/* Set initial DC values */
> @@ -1106,15 +1104,16 @@ static void init_rog_tunables(void)
>  		pr_debug("No DC PPT limits defined\n");
>  	}
>  
> -	return;
> +	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = ac_rog_tunables;
> +	asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
>  
> -err_nomem:
> -	pr_err("Failed to allocate memory for tunables\n");
> +	return 0;
>  }
>  
>  static int __init asus_fw_init(void)
>  {
>  	char *wmi_uid;
> +	int err;
>  
>  	wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
>  	if (!wmi_uid)
> @@ -1127,10 +1126,21 @@ static int __init asus_fw_init(void)
>  	if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
>  		return -ENODEV;
>  
> -	init_rog_tunables();
> +	err = init_rog_tunables();
> +	if (err)
> +		return err;
>  
>  	/* Must always be last step to ensure data is available */
> -	return asus_fw_attr_add();
> +	err = asus_fw_attr_add();
> +	if (err)
> +		goto err_free_tunables;
> +
> +	return 0;
> +
> +err_free_tunables:
> +	kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
> +	kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
> +	return err;
>  }
>  
>  static void __exit asus_fw_exit(void)
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-07 16:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-03 10:31 [PATCH v3 0/2] platform/x86: asus-armoury: fix init errors and modernize resource management Marco Scardovi
2026-07-03 10:31 ` [PATCH v3 1/2] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init Marco Scardovi
2026-07-07 16:54   ` Ilpo Järvinen
2026-07-03 10:31 ` [PATCH v3 2/2] platform/x86: asus-armoury: use cleanup.h to manage tunables Marco Scardovi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®