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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 1 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().
> 
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>
> ---
> v2:
> - Restructure init_rog_tunables() to use local pointers and defer assignment
>   to the global struct until all allocations succeed, eliminating the need
>   to set global pointers to NULL on error.
> - Use a goto-rollback pattern in asus_fw_init() for cleaner error handling
>   as requested by Ilpo.
> 
>  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;

This works as well but now we also want to convert it to use __free() 
+ no_free_ptr() so we can get rid of that manual kfree() to remove 
potential for future mem management errors.

...If you could add a second patch to this series to that effect, it would 
great.

--
 i.

>  }
>  
>  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)
>