Re: [PATCH] hwmon: (minisforum-um780xtx) Fix clamp() build failure with GCC 11

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hwmon: minisforum-um780xtx: integer overflow in DIV_ROUND_CLOSEST
--

commit 37b4941cfec26b0afeccdcd44aef6f32fe1e9d55
Author: Sebastián Peyrott <speyrott@gmail.com>

hwmon: (minisforum-um780xtx) Fix clamp() build failure with GCC 11

This patch fixes a GCC 11 build failure by selecting clamping limits early
and using a single clamp_val() execution. This prevents GCC jump threading
from duplicating the limit check and firing an unreachable compile-time
assertion.

Closes: https://lore.kernel.org/r/fda0e59f-20e1-4a94-a714-288940dcdd37@paulmck-laptop

> diff --git a/drivers/hwmon/minisforum-um780xtx.c b/drivers/hwmon/minisforum-um780xtx.c
> index f5d6e18cb651f..962ebf41ffb7f 100644
> --- a/drivers/hwmon/minisforum-um780xtx.c
> +++ b/drivers/hwmon/minisforum-um780xtx.c

[ ... ]

> @@ -198,13 +198,15 @@ static ssize_t um780xtx_sys_point_temp_store(struct device *dev,
>  	if (points[0] >= points[1] || points[1] >= data->sys_point3)
>  		return -EIO;
>  
> +	if (!sattr->index) {
> +		lo = 0;
> +		hi = points[1] - 1;
> +	} else {
> +		lo = points[0] + 1;
> +		hi = data->sys_point3 - 1;
> +	}
>  	value = DIV_ROUND_CLOSEST(value, 1000);
> -	if (!sattr->index)
> -		value = clamp_val(value, 0, points[1] - 1);
> -	else
> -		value = clamp_val(value, points[0] + 1,
> -				  data->sys_point3 - 1);
> -	points[sattr->index] = value;
> +	points[sattr->index] = clamp_val(value, lo, hi);

[Severity: Low]
Can this arithmetic result in an integer overflow for extreme inputs?

When a user writes an extremely large value like LONG_MAX to the sysfs file
in um780xtx_sys_point_temp_store(), the unbounded value is passed directly
to DIV_ROUND_CLOSEST(value, 1000) before it is clamped. 

Because DIV_ROUND_CLOSEST adds half the divisor before dividing, an extreme
positive input can overflow and wrap around to a negative number. This would
cause the value to incorrectly clamp to the minimum bound (lo) instead of
the maximum bound (hi). Conversely, an extreme negative input could underflow
and improperly clamp to hi.

Would it be safer to pre-clamp the value before calling DIV_ROUND_CLOSEST
to prevent this overflow? Many other hwmon drivers clamp the input bounds
first prior to performing the division to avoid this behavior.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014214522.243253-1-speyrott@gmail.com?part=1