Re: [PATCH 2/3] hwmon: lm93: Change unsigned to unsigned int for clarity

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Broken arithmetic and unsigned underflow in `LM93_IN_REL_TO_REG` completely breaks relative voltage limit configuration.
--

commit d027c76dfc6bdf77f6a91f44199f1d10dda57fdd
Author: Ninad Naik <ninadnaik07@gmail.com>

hwmon: lm93: Change unsigned to unsigned int for clarity

This commit fixes checkpatch.pl warnings by changing unsigned to unsigned int
for clarity and to follow the latest kernel code style.

> diff --git a/drivers/hwmon/lm93.c b/drivers/hwmon/lm93.c
> index c15e2a02dd86a..1a3b5da924b2e 100644
> --- a/drivers/hwmon/lm93.c
> +++ b/drivers/hwmon/lm93.c

[ ... ]

> @@ -396,7 +396,7 @@ static unsigned LM93_IN_REL_FROM_REG(u8 reg, int upper, int vid)
>   * upper also determines which nibble of the register is returned
>   * (the other nibble will be 0x0)
>   */
> -static u8 LM93_IN_REL_TO_REG(unsigned val, int upper, int vid)
> +static u8 LM93_IN_REL_TO_REG(unsigned int val, int upper, int vid)
>  {
>  	long uv_offset = vid * 1000 - val * 10000;

This isn't a regression introduced by this patch, but does this calculation
cause an unsigned underflow?

Because val is an unsigned int, the operation vid * 1000 - val * 10000 is
evaluated as unsigned. For an upper limit, the limit (val * 10000) is
typically greater than the voltage (vid * 1000), causing the subtraction to
wrap around to a large positive value.

On 64-bit systems, assigning this to long uv_offset zero-extends it. The
function then executes clamp_val(uv_offset, 12500, 200000). Because the
underflowed value is massive, it is clamped to the maximum bound (200000).
On 32-bit systems, the huge unsigned value is reinterpreted as negative and
clamped to the minimum bound (12500). For lower limits, the inverted
subtraction yields a positive value, which is then clamped against a
negative range (-400000 to -25000), forcing it to -25000.

Also, does the formula subtract in the wrong order? To correctly invert the
logic of LM93_IN_REL_FROM_REG (which computes
val = (vid * 1000 + uv_offset + 5000) / 10000), should the offset be
calculated as uv_offset = val * 10000 - vid * 1000?

Lastly, since the unbounded user input val is multiplied by 10000 prior to
any bounds checking, can large inputs silently overflow the 32-bit unsigned
integer?

Could these arithmetic regressions cause writes to the in_min or in_max
sysfs attributes for channels 6 and 7 to misconfigure the hardware
monitoring bounds, potentially masking important voltage alarms?

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