Re: [PATCH v4 2/8] hwmon: sbtsi_temp: Refactor temperature register access into helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Setting an extremely high temperature limit in extended range mode causes a signed integer overflow, resulting in the limit being incorrectly clamped to the minimum temperature.
--

--- Patch [2]: [PATCH v4 2/8] hwmon: sbtsi_temp: Refactor temperature register access into helpers ---
commit 64811eaa450f0343a3d3098341243ec7d1df489c
Author: Prathima <Prathima.Lk@amd.com>

hwmon: sbtsi_temp: Refactor temperature register access into helpers

This commit extracts the paired integer and decimal register reads and writes
from the hwmon callbacks into helper functions. It consolidates error
handling and respects the ReadOrder bit to ensure atomic latching.

> diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> index 28258bf499223..078f4ab25bde5 100644
> --- a/drivers/hwmon/sbtsi_temp.c
> +++ b/drivers/hwmon/sbtsi_temp.c

[ ... ]

> @@ -127,11 +169,7 @@ static int sbtsi_write(struct device *dev, enum hwmon_sensor_types type,
>  	val = clamp_val(val, SBTSI_TEMP_MIN, SBTSI_TEMP_MAX);
>  	sbtsi_mc_to_reg(val, &temp_int, &temp_dec);
>  
> -	err = i2c_smbus_write_byte_data(data->client, reg_int, temp_int);
> -	if (err)
> -		return err;
> -
> -	return i2c_smbus_write_byte_data(data->client, reg_dec, temp_dec);
> +	return sbtsi_temp_write(data, reg_int, reg_dec, temp_int, temp_dec);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but can val overflow right before
the clamping happens in sbtsi_write()?

Looking at drivers/hwmon/sbtsi_temp.c around line 168, just above this
diff context:

	if (data->ext_range_mode)
		val += SBTSI_TEMP_EXT_RANGE_ADJ;
	val = clamp_val(val, SBTSI_TEMP_MIN, SBTSI_TEMP_MAX);

If a privileged user writes an extremely high value to the sysfs temperature
limit file (for example, a value near LONG_MAX), adding
SBTSI_TEMP_EXT_RANGE_ADJ to val could cause a signed integer overflow.

This would wrap to a negative value, causing clamp_val() to incorrectly
evaluate it as being below the lower bound and clamp it to SBTSI_TEMP_MIN
instead of SBTSI_TEMP_MAX.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710111642.850022-1-Akshay.Gupta@amd.com?part=2