[PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[Zilin Guan]

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@linux.intel.com>
Suggested-by: Markus Elfring <Markus.Elfring@web.de>
Co-developed-by: Jianhao Xu <jianhao.xu@seu.edu.cn>
Signed-off-by: Jianhao Xu <jianhao.xu@seu.edu.cn>
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
Changes in v2:
- using scope-based cleanup helper __free() for automatic resource cleanup.

 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;
 }
 
-- 
2.34.1

Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[Markus Elfring]

…> +++ 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)
>  {
…

Is there a need to move such a special macro call into an other source (or header) file?

Regards,
Markus

Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[Zilin Guan]

On Sat, Jan 03, 2026 at 02:16:12PM +0100, Markus Elfring wrote:
> …> +++ 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)
> >  {
> …
> 
> Is there a need to move such a special macro call into an other source (or header) file?
> 
> Regards,
> Markus

Currently, this helper is only utilized in this function. I believe 
keeping it local is appropriate for this bug fix. We can move it to a 
generic header if more use cases arise in the future.

Regards,
Zilin Guan

Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3244 bytes --]

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@linux.intel.com>
> Suggested-by: Markus Elfring <Markus.Elfring@web.de>
> Co-developed-by: Jianhao Xu <jianhao.xu@seu.edu.cn>
> Signed-off-by: Jianhao Xu <jianhao.xu@seu.edu.cn>
> Signed-off-by: Zilin Guan <zilin@seu.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;
>  }
>  
> 

Re: [PATCH v2] platform/x86/amd: Fix memory leak in wbrf_record()

[Zilin Guan]

On Mon, Jan 05, 2026 at 03:52:14PM +0200, "Ilpo Järvinen" wrote:
> 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.

Hi Ilpo,

Thanks for your review.

I will follow your suggestion to split the patch into two in v3.

Just for context, the reason I modified 'obj' in v2 is to avoid mixing 
goto-based cleanup with scope-based cleanup, as recommended in 
cleanup.h [1]. This was also why I stuck to goto in v1 
(to avoid touching 'obj'). 

[1] https://elixir.bootlin.com/linux/v6.19-rc4/source/include/linux/cleanup.h#L148-L153

Best regards,
Zilin Guan