Re: [PATCH] hwmon: (w83627ehf) Clamp voltage limits before scaling

[David Laight]

On Wed, 11 Nov 2026 14:48:31 +0800
Zhu Jun <zhujun2@xxxxxxxxxxxxxxxxxxxx> wrote:

> in_to_reg() narrows the sysfs value to u32 and multiplies it by 100
> before clamping. A large positive voltage limit can consequently be
> programmed as zero instead of the maximum; 4294967296 mV reproduces this
> on a 64-bit system.
> 
> Preserve the input width and clamp to the representable voltage range
> before multiplication.
> 
> Fixes: cf0676fe42c8 ("[PATCH] w83627ehf: Add voltage inputs support")
> Signed-off-by: Zhu Jun <zhujun2@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/hwmon/w83627ehf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
> index 9c08ab662dc6..5c04b9c52b76 100644
> --- a/drivers/hwmon/w83627ehf.c
> +++ b/drivers/hwmon/w83627ehf.c
> @@ -293,8 +293,9 @@ static inline long in_from_reg(u8 reg, u8 nr, const u16 *scale_in)
>  	return DIV_ROUND_CLOSEST(reg * scale_in[nr], 100);
>  }
>  
> -static inline u8 in_to_reg(u32 val, u8 nr, const u16 *scale_in)
> +static inline u8 in_to_reg(unsigned long val, u8 nr, const u16 *scale_in)
>  {
> +	val = min(val, 255UL * scale_in[nr] / 100);

You need to think about these more.
Either it really doesn't matter because no one would really try to write
an over large value unless they are trying to break things - and they can
just write zero, or you just need to stop the n * 100 + x overflowing.
So at least avoid the extra divide by using:
	val = min(val, (~0ul - 32768) / 100) 

>  	return clamp_val(DIV_ROUND_CLOSEST(val * 100, scale_in[nr]), 0, 255);

Another clamp_val() that should never have been added.
The type checks are there for a reason, not to be killed by casts.

David

>  }
>