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

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

[Yuho Choi]

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 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include "acpi/actypes.h"
 #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);
 	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) {
-- 
2.50.1 (Apple Git-155)

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

[Zhongqiu Han]

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

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

[최유호]

Dear Zhongqiu Han,

> 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?

You are right, it is unnecessary. I will drop it in v2.

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

Agreed, that is cleaner and makes the intent explicit. I will adopt
your suggestion in v2.

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

Noted. I will address those in a separate patch.

Thanks for the review.

Best regards,
Yuho Choi

On Thu, 16 Apr 2026 at 02:04, Zhongqiu Han
<zhongqiu.han@oss.qualcomm.com> wrote:
>
> 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