linux-kernel - Re: thermal/drivers/airoha: Fix low trip temperature setting

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: <6925bcba.df0a0220.925a7.ca27@mx.google.com>
Date: Tue, 25 Nov 2025 15:27:02 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: david laight <david.laight@...box.com>
Cc: rafael@...nel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: thermal/drivers/airoha: Fix low trip temperature setting

On Sat, Nov 22, 2025 at 10:07:47PM +0000, david laight wrote:
> There is a 'cut&paste' error in the code that sets the 'low' limit.
> The clamp() calls contains 'high' instead of 'low'.
> 
> While FEILD_MAX() ought to be (and probably is) a compile-time constant.
> it isn't constant enough for the range check I'm adding to clamp_t().
> Additionally (and probably accidentally) it has a 64bit unsigned type
> which propagates through RAW_TO_TEMP() causing a 'signedness error'.
> Hit it at the root cause with an (int) cast of FIELD_MAX().
> (Alternatively arrange for RAW_TO_TEMP() to always return a signed value.)
>

Sorry but I'm not very convinced by this section.

clamp_t in theory cast everything to the requested type. FIELD_MAX will
correctly result in the max value for the register and then RAW_TO_TEMP
should convert the value to a temp and be an int.

If it wasn't the case it should have been a BUILD_BUG error.

Am I missing something?

> Fixes: 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")
> 
> Signed-off by: David Laight <david.laight.linux@...il.com>
> 
> ---
> This is a diff because I've not even contemplated committing the nearly 500
> files I changed.
> 
> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index b9fd6bfc88e5..75afc94fea5f 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
> @@ -261,8 +261,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  
>  	if (high != INT_MAX) {
>  		/* Validate high and clamp it a supported value */
> -		high = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
> -			       RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
> +		high = clamp(high, RAW_TO_TEMP(priv, 0),
> +			       RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
>  
>  		/* We offset the high temp of 1°C to trigger correct event */
>  		writel(TEMP_TO_RAW(priv, high) >> 4,
> @@ -273,8 +273,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  
>  	if (low != -INT_MAX) {
>  		/* Validate low and clamp it to a supported value */
> -		low = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
> -			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
> +		low = clamp(low, RAW_TO_TEMP(priv, 0),
> +			      RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
>  
>  		/* We offset the low temp of 1°C to trigger correct event */
>  		writel(TEMP_TO_RAW(priv, low) >> 4,
> 

-- 
	Ansuel