[PATCH] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init

[PATCH] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init

[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(). On failure, make sure all partially
allocated tunables are freed and their pointers are cleared to NULL.

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

diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
index 495dc1e31d40..3a63f960eb3f 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,14 +1000,14 @@ 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 */
@@ -1015,7 +1015,7 @@ static void init_rog_tunables(void)
 	if (ac_limits) {
 		ac_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
 		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;
@@ -1063,7 +1063,8 @@ static void init_rog_tunables(void)
 		dc_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
 		if (!dc_rog_tunables) {
 			kfree(ac_rog_tunables);
-			goto err_nomem;
+			asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = NULL;
+			return -ENOMEM;
 		}
 
 		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
@@ -1106,15 +1107,13 @@ static void init_rog_tunables(void)
 		pr_debug("No DC PPT limits defined\n");
 	}
 
-	return;
-
-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,19 @@ 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) {
+		kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
+		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = NULL;
+		kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
+		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = NULL;
+	}
+
+	return err;
 }
 
 static void __exit asus_fw_exit(void)
-- 
2.54.0

Re: [PATCH] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init

[Ilpo Järvinen]

On Sun, 14 Jun 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(). On failure, make sure all partially
> allocated tunables are freed and their pointers are cleared to NULL.
> 
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>
> ---
>  drivers/platform/x86/asus-armoury.c | 32 ++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 495dc1e31d40..3a63f960eb3f 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,14 +1000,14 @@ 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 */
> @@ -1015,7 +1015,7 @@ static void init_rog_tunables(void)
>  	if (ac_limits) {
>  		ac_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
>  		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;
> @@ -1063,7 +1063,8 @@ static void init_rog_tunables(void)
>  		dc_rog_tunables = kzalloc_obj(*asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
>  		if (!dc_rog_tunables) {
>  			kfree(ac_rog_tunables);
> -			goto err_nomem;
> +			asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = NULL;
> +			return -ENOMEM;
>  		}
>  
>  		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = dc_rog_tunables;
> @@ -1106,15 +1107,13 @@ static void init_rog_tunables(void)
>  		pr_debug("No DC PPT limits defined\n");
>  	}
>  
> -	return;
> -
> -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,19 @@ 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) {
> +		kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]);
> +		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] = NULL;
> +		kfree(asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC]);
> +		asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC] = NULL;

Why do you need to set these to NULL?

I'd also prefer to use goto + rollback path pattern even if there's only 
one entry at this point.

-- 
 i.

Re: [PATCH] platform/x86: asus-armoury: fix Use-After-Free and memory leak in driver init

[Marco Scardovi]

In data mercoledì 1 luglio 2026 13:01:12 Ora legale dell’Europa centrale, Ilpo Järvinen ha scritto:
> On Sun, 14 Jun 2026, Marco Scardovi wrote:
> 
> > ...
> 
> Why do you need to set these to NULL?
> 
> I'd also prefer to use goto + rollback path pattern even if there's only 
> one entry at this point.
> 
Hi Ilpo,

thanks for looking into it. If I have to be completely honest is because
I didn't think at it as a solution: I'd also prefer to use goto + rollback.
I'll open a new thread with your suggestion.

With best regards,
Marco