linux-kernel - Re: [PATCH] w1: therm: Use clamp_t to simplify int_to

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251109162056.0a9cbd52@pumpkin>
Date: Sun, 9 Nov 2025 16:20:56 +0000
From: David Laight <david.laight.linux@...il.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Huisong Li
 <lihuisong@...wei.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] w1: therm: Use clamp_t to simplify int_to_short helper

On Sun,  9 Nov 2025 13:59:55 +0100
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> Use clamp_t() instead of manually casting the return value.
> 
> Replace sprintf() with sysfs_emit() to improve sysfs show functions
> while we're at it.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
>  drivers/w1/slaves/w1_therm.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 9ccedb3264fb..cf686e6ba3d5 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -961,9 +961,8 @@ static inline int temperature_from_RAM(struct w1_slave *sl, u8 rom[9])
>   */
>  static inline s8 int_to_short(int i)
>  {
> -	/* Prepare to cast to short by eliminating out of range values */
> -	i = clamp(i, MIN_TEMP, MAX_TEMP);
> -	return (s8) i;
> +	/* Cast to short by eliminating out of range values */
                   ^^^^^ no shorts here...
> +	return clamp_t(s8, i, MIN_TEMP, MAX_TEMP);

That is just plain broken.
clamp_t() really shouldn't have been allowed to exist.
That is a typical example of how it gets misused.
(min_t() and max_t() get misused the same way.)

Think what happens when i is 256.
The code should just be:

	return clamp(i, MIN_TEMP, MAX_TEMP);

No casts anywhere.
I'm not even sure the return type (s8) makes any sense.
It is quite likely that the code will be better if it is 'int'.
The fact that the domain in inside -128..127 doesn't mean that
the correct type for a variable isn't 'int'.

	David

>  }