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

[zhenglifeng (A)]

On 3/30/2026 12:00 PM, Zhongqiu Han wrote:
> 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;
> 

Therefore, it seems the only option is to allocate memory separately for
boost_freq_req.