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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >   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