linux-kernel - Re: [PATCH v1 2/2] PM: devfreq: handle invalid parameters gracefully in simpleondemand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b832176d-dc48-4c5b-ae69-aca885667cc0@kylinos.cn>
Date: Wed, 26 Nov 2025 17:24:21 +0800
From: luriwen <luriwen@...inos.cn>
To: Chanwoo Choi <chanwoo@...nel.org>, myungjoo.ham@...sung.com,
 kyungmin.park@...sung.com, cw00.choi@...sung.com
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] PM: devfreq: handle invalid parameters gracefully
 in simpleondemand

在 2025/11/23 23:24, Chanwoo Choi 写道:
> 25. 11. 18. 12:23에 Riwen Lu 이(가) 쓴 글:
>> Instead of returning -EINVAL when upthreshold > 100 or upthreshold <
>> downdifferential, fall back to default threshold values to ensure the
>> governor continues functioning.
>>
>> Additionally, the validation is now scoped to the if (data) block,
>> preventing unnecessary checks when no user data is provided, while the
>> fallback mechanism ensures reliability with invalid configurations.
>>
>> Signed-off-by: Riwen Lu <luriwen@...inos.cn>
>> ---
>>   drivers/devfreq/governor_simpleondemand.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> index b4d7be766f33..7205891d2ec6 100644
>> --- a/drivers/devfreq/governor_simpleondemand.c
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -36,10 +36,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>   			dfso_upthreshold = data->upthreshold;
>>   		if (data->downdifferential)
>>   			dfso_downdifferential = data->downdifferential;
>> +
>> +		if (dfso_upthreshold > 100 ||
>> +		    dfso_upthreshold < dfso_downdifferential) {
>> +			dfso_upthreshold = DFSO_UPTHRESHOLD;
>> +			dfso_downdifferential = DFSO_DOWNDIFFERENTIAL;
>> +			pr_debug("Invalid thresholds, using defaults: up = %u, down = %u\n",
>> +				dfso_upthreshold, dfso_downdifferential);
>> +		}
>>   	}
>> -	if (dfso_upthreshold > 100 ||
>> -	    dfso_upthreshold < dfso_downdifferential)
>> -		return -EINVAL;
>>   
>>   	/* Assume MAX if it is going to be divided by zero */
>>   	if (stat->total_time == 0) {
> If there are wrong initialization of devfreq_simple_ondemand,
> it should point out what is wrong because it makes some confusion if there are no error.
>
>
The original intention behind falling back to default values was to ensure the governor remains functional even with invalid configurations. However, I agree that silently using defaults could hide configuration issues from users.

I'd like to keep fallback but add explicit warning or error.

```c

if (dfso_upthreshold > 100 ||
     dfso_upthreshold < dfso_downdifferential) {
         pr_warn("Invalid thresholds (up = %u, down = %u), using 
defaults: up = %u, down = %u\n",
                 dfso_upthreshold, dfso_downdifferential,
                 DFSO_UPTHRESHOLD, DFSO_DOWNDIFFERENTIAL);
         dfso_upthreshold = DFSO_UPTHRESHOLD;
         dfso_downdifferential = DFSO_DOWNDIFFERENTIAL;
}

--

Best Regards,

Riwen Lu