Re: [PATCH v8 2/2] cpufreq: Add boost_freq_req QoS request

[Zhongqiu Han]

On 3/30/2026 10:10 AM, zhenglifeng (A) wrote:
> On 3/29/2026 5:00 PM, Zhongqiu Han wrote:
> > > @@ -1377,6 +1386,7 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> > >        }
> > >          freq_qos_remove_request(policy->min_freq_req);
> > > +    freq_qos_remove_request(policy->boost_freq_req);
> > >        kfree(policy->min_freq_req);
> > >          cpufreq_policy_put_kobj(policy);
> > > @@ -1445,26 +1455,38 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
> > >        cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
> > >          if (new_policy) {
> > > +        unsigned int count;
> > > +
> > >            for_each_cpu(j, policy->related_cpus) {
> > >                per_cpu(cpufreq_cpu_data, j) = policy;
> > >                add_cpu_dev_symlink(policy, j, get_cpu_device(j));
> > >            }
> > >    -        policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> > > +        count = policy->boost_supported ? 3 : 2;
> > > +        policy->min_freq_req = kzalloc(count * sizeof(*policy->min_freq_req),
> > >                               GFP_KERNEL);
> > >            if (!policy->min_freq_req) {
> > >                ret = -ENOMEM;
> > >                goto out_destroy_policy;
> > >            }
> > >    +        if (policy->boost_supported) {
> > > +            policy->boost_freq_req = policy->min_freq_req + 2;
> > > +
> > > +            ret = freq_qos_add_request(&policy->constraints,
> > > +                           policy->boost_freq_req,
> > > +                           FREQ_QOS_MAX,
> > > +                           policy->cpuinfo.max_freq);
> > > +            if (ret < 0) {
> > > +                policy->boost_freq_req = NULL;
> > > +                goto out_destroy_policy;
> > > +            }
> > > +        }
> > > +
> > >            ret = freq_qos_add_request(&policy->constraints,
> > >                           policy->min_freq_req, FREQ_QOS_MIN,
> > >                           FREQ_QOS_MIN_DEFAULT_VALUE);
> > >            if (ret < 0) {
> > > -            /*
> > > -             * So we don't call freq_qos_remove_request() for an
> > > -             * uninitialized request.
> > > -             */
> > >                kfree(policy->min_freq_req);
> > >                policy->min_freq_req = NULL;
> > >                goto out_destroy_policy;
> >
> > Hi Pierre, Viresh,
> >
> > Sorry for the late follow-up on v8. While re-reading the patch, I
> > noticed a potential UAF issue on an error path — I might be missing
> > something, so I'd appreciate a double-check.
> >
> > min_freq_req, max_freq_req and boost_freq_req all point into the same
> > contiguous kzalloc'd block:
> >
> > slot0 (min_freq_req + 0) -> min_freq_req
> > slot1 (min_freq_req + 1) -> max_freq_req
> > slot2 (min_freq_req + 2) -> boost_freq_req
> >
> > If boost_freq_req is successfully added to the QoS constraints list, but
> > the subsequent freq_qos_add_request() for min_freq_req fails, the error
> > path does:
> >
> > kfree(policy->min_freq_req); /* frees the entire block, including slot2
> > */
> > policy->min_freq_req = NULL;
> > goto out_destroy_policy;
> >
> > policy->boost_freq_req is not set to NULL here, so it becomes a dangling
> > pointer into freed memory.
> > cpufreq_policy_free() is then called from cpufreq_online() and does:
> >
> > freq_qos_remove_request(policy->boost_freq_req); /* UAF */
> > or this boost qos req will leak.
>
> Good catch!
>
> How about remove the kfree() here and just leave it to
> cpufreq_policy_free()?

Thanks for the suggestion — this is another fix approach we can
explore, but there seems to be a small caveat.

Some additional changes would still be needed; otherwise, removing the
kfree() here and deferring it to cpufreq_policy_free() can lead to a
warning.

The reason is that we neither free policy->min_freq_req nor set policy
->min_freq_req = NULL. As a result, when cpufreq_policy_free() later
calls freq_qos_remove_request(policy->min_freq_req), it hits the
following warning:

if (WARN(!freq_qos_request_active(req),
	"%s() called for unknown object\n", __func__))
	return -EINVAL;




--
Thx and BRs,
Zhongqiu Han