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

[zhenglifeng (A)]

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