Re: [PATCH v1] cpufreq: pcc: fix use-after-free and double free in _OSC evaluation

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 4/16/2026 12:51 AM, Yuho Choi wrote:
> pcc_cpufreq_do_osc() evaluates _OSC twice with the same output buffer.
> The first acpi_evaluate_object() allocates the buffer because output is
> initialized with ACPI_ALLOCATE_BUFFER. Freeing output.pointer before the
> second evaluation leaves output.length stale, so the next call treats
> output as a caller-supplied buffer and performs a use-after-free write
> into the freed memory. The final cleanup path then frees the same
> pointer again, causing a double free.
> 
> Keep the first _OSC result alive until the shared cleanup path and route
> the early error exits through out_free. This avoids both the use-after-
> free on the second evaluation and the final double free.
> 
> Fixes: 0f1d683fb35d ("[CPUFREQ] Processor Clocking Control interface driver")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---
>   drivers/cpufreq/pcc-cpufreq.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> index ac2e90a65f0c4..165826b5d6844 100644
> --- a/drivers/cpufreq/pcc-cpufreq.c
> +++ b/drivers/cpufreq/pcc-cpufreq.c
> @@ -23,6 +23,7 @@
>    * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    */

Hi Yuho Choi,
Thanks for the patch.

>   
> +#include "acpi/actypes.h"

1.I don't see why this header is needed for this change. Even if it is,
it’s already included by <linux/acpi.h>, right?

>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/init.h>
> @@ -351,16 +352,19 @@ static int __init pcc_cpufreq_do_osc(acpi_handle *handle)
>   		goto out_free;
>   	}
>   
> -	kfree(output.pointer);

2.Would it be cleaner to reset the pointer and output.length, and let
acpi_evaluate_object() reallocate the buffer? For example:

         kfree(output.pointer);
+       output.pointer = NULL;
+       output.length = ACPI_ALLOCATE_BUFFER;



3.The sashiko.dev pointed out a few pre-existing boundary checking
issues that are outside the scope of this patch.

https://sashiko.dev/#/patchset/20260415165139.14113-1-dbgh9129%40gmail.com

>   	capabilities[0] = 0x0;
>   	capabilities[1] = 0x1;
>   
>   	status = acpi_evaluate_object(*handle, "_OSC", &input, &output);
> -	if (ACPI_FAILURE(status))
> -		return -ENODEV;
> +	if (ACPI_FAILURE(status)) {
> +		ret = -ENODEV;
> +		goto out_free;
> +	}
>   
> -	if (!output.length)
> -		return -ENODEV;
> +	if (!output.length) {
> +		ret = -ENODEV;
> +		goto out_free;
> +	}
>   
>   	out_obj = output.pointer;
>   	if (out_obj->type != ACPI_TYPE_BUFFER) {


-- 
Thx and BRs,
Zhongqiu Han