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

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

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