Re: [PATCH v3] platform/x86: intel_pmc_ipc: fix ACPI buffer memory leak

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/11/2025 04:32, yongxin.liu@windriver.com wrote:
> From: Yongxin Liu <yongxin.liu@windriver.com>
> 
> The intel_pmc_ipc() function uses ACPI_ALLOCATE_BUFFER to allocate memory
> for the ACPI evaluation result but never frees it, causing a 192-byte
> memory leak on each call.
> 
> This leak is triggered during network interface initialization when the
> stmmac driver calls intel_mac_finish() -> intel_pmc_ipc().
> 
>   unreferenced object 0xffff96a848d6ea80 (size 192):
>     comm "dhcpcd", pid 541, jiffies 4294684345
>     hex dump (first 32 bytes):
>       04 00 00 00 05 00 00 00 98 ea d6 48 a8 96 ff ff  ...........H....
>       00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00  ................
>     backtrace (crc b1564374):
>       kmemleak_alloc+0x2d/0x40
>       __kmalloc_noprof+0x2fa/0x730
>       acpi_ut_initialize_buffer+0x83/0xc0
>       acpi_evaluate_object+0x29a/0x2f0
>       intel_pmc_ipc+0xfd/0x170
>       intel_mac_finish+0x168/0x230
>       stmmac_mac_finish+0x3d/0x50
>       phylink_major_config+0x22b/0x5b0
>       phylink_mac_initial_config.constprop.0+0xf1/0x1b0
>       phylink_start+0x8e/0x210
>       __stmmac_open+0x12c/0x2b0
>       stmmac_open+0x23c/0x380
>       __dev_open+0x11d/0x2c0
>       __dev_change_flags+0x1d2/0x250
>       netif_change_flags+0x2b/0x70
>       dev_change_flags+0x40/0xb0
> 
> Add __free(kfree) for ACPI object to properly release the allocated buffer.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7e2f7e25f6ff ("arch: x86: add IPC mailbox accessor function and add SoC register access")
> Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
> ---
> V2->V3:
> Use __free(kfree) instead of goto and kfree();
> 
> V1->V2:
> Cover all potential paths for kfree();
> ---
>  include/linux/platform_data/x86/intel_pmc_ipc.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/platform_data/x86/intel_pmc_ipc.h b/include/linux/platform_data/x86/intel_pmc_ipc.h
> index 1d34435b7001..cf0b78048b0e 100644
> --- a/include/linux/platform_data/x86/intel_pmc_ipc.h
> +++ b/include/linux/platform_data/x86/intel_pmc_ipc.h
> @@ -9,6 +9,7 @@
>  #ifndef INTEL_PMC_IPC_H
>  #define INTEL_PMC_IPC_H
>  #include <linux/acpi.h>
> +#include <linux/cleanup.h>
>  
>  #define IPC_SOC_REGISTER_ACCESS			0xAA
>  #define IPC_SOC_SUB_CMD_READ			0x00
> @@ -48,7 +49,7 @@ static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf
>  		{.type = ACPI_TYPE_INTEGER,},
>  	};
>  	struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params };
> -	union acpi_object *obj;
> +	union acpi_object *obj __free(kfree) = NULL;


This is undesired syntax explicitly documented as one to avoid. Please
don't use cleanup.h if you do not intend to follow it because it does
not make the code simpler. The rule of explicit (useful, not NULL)
constructor

Best regards,
Krzysztof