linux-kernel - Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <513faa30-9241-49d4-873a-96ffdd314504@linux.intel.com>
Date: Mon, 5 Jan 2026 15:52:14 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Zilin Guan <zilin@....edu.cn>
cc: Hans de Goede <hansg@...nel.org>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>, Markus.Elfring@....de, 
    Jianhao Xu <jianhao.xu@....edu.cn>
Subject: Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

On Sat, 3 Jan 2026, Zilin Guan wrote:

> The tmp buffer is allocated using kcalloc() but is not freed if
> acpi_evaluate_dsm() fails. This causes a memory leak in the error path.
> 
> Fix this by using the scope-based cleanup helper __free() for automatic
> resource cleanup. This ensures that both the tmp buffer and the ACPI
> object are automatically freed when they go out of scope, simplifying
> error handling and preventing leaks.
> 
> Fixes: 58e82a62669d ("platform/x86/amd: Add support for AMD ACPI based Wifi band RFI mitigation feature")\
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> Suggested-by: Markus Elfring <Markus.Elfring@....de>
> Co-developed-by: Jianhao Xu <jianhao.xu@....edu.cn>
> Signed-off-by: Jianhao Xu <jianhao.xu@....edu.cn>
> Signed-off-by: Zilin Guan <zilin@....edu.cn>
> ---
> Changes in v2:
> - using scope-based cleanup helper __free() for automatic resource cleanup.

While I'm certainly not against in converting also 'obj', this patch 
should be split to two, where the first patch fixes the case that is 
current lacking kfree(), and the second that does convert 'obj'. IIRC, on 
can just use kfree() directly for releasing it so adding the new 
DEFINE_FREE() doesn't seem necessary.

-- 
 i.

>  drivers/platform/x86/amd/wbrf.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/wbrf.c b/drivers/platform/x86/amd/wbrf.c
> index dd197b3aebe0..4517d139d768 100644
> --- a/drivers/platform/x86/amd/wbrf.c
> +++ b/drivers/platform/x86/amd/wbrf.c
> @@ -39,11 +39,11 @@ static const guid_t wifi_acpi_dsm_guid =
>   */
>  static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
>  
> +DEFINE_FREE(acpi_object, union acpi_object *, if (_T) ACPI_FREE(_T))
> +
>  static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ranges_in_out *in)
>  {
>  	union acpi_object argv4;
> -	union acpi_object *tmp;
> -	union acpi_object *obj;
>  	u32 num_of_ranges = 0;
>  	u32 num_of_elements;
>  	u32 arg_idx = 0;
> @@ -74,7 +74,7 @@ static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ran
>  	 */
>  	num_of_elements = 2 * num_of_ranges + 2;
>  
> -	tmp = kcalloc(num_of_elements, sizeof(*tmp), GFP_KERNEL);
> +	union acpi_object *tmp __free(kfree) = kcalloc(num_of_elements, sizeof(*tmp), GFP_KERNEL);
>  	if (!tmp)
>  		return -ENOMEM;
>  
> @@ -101,25 +101,20 @@ static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ran
>  		tmp[arg_idx++].integer.value = in->band_list[i].end;
>  	}
>  
> -	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
> -				WBRF_REVISION, WBRF_RECORD, &argv4);
> +	union acpi_object *obj __free(acpi_object) =
> +		acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +				  WBRF_REVISION, WBRF_RECORD, &argv4);
>  
>  	if (!obj)
>  		return -EINVAL;
>  
> -	if (obj->type != ACPI_TYPE_INTEGER) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> +	if (obj->type != ACPI_TYPE_INTEGER)
> +		return -EINVAL;
>  
>  	ret = obj->integer.value;
>  	if (ret)
>  		ret = -EINVAL;
>  
> -out:
> -	ACPI_FREE(obj);
> -	kfree(tmp);
> -
>  	return ret;
>  }
>  
>