Re: [PATCH v2 1/2] hwmon: (max6621) fix temperature clamp range

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

Pre-existing issues:
- [High] Reading a channel-specific alarm attribute returns the global alert status and silently clears alerts for all channels.
--

--- Patch [1]: [PATCH v2 1/2] hwmon: (max6621) fix temperature clamp range ---
commit 6a78b24a06173620d98274b9f9d004f988e65833
Author: Cong Nguyen <[email protected]>

hwmon: (max6621) fix temperature clamp range

MAX6621_TEMP_INPUT_MIN and MAX6621_TEMP_INPUT_MAX are used to clamp the
writable offset and critical thresholds. They are defined as -127000 and
128000. Correct the limits to -128000 and 127000.

Fixes: 92b64580f14b ("hwmon: (max6621) Add support for Maxim MAX6621 temperature sensor")
Cc: [email protected]
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <[email protected]>

> diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
> index e86ec6d237ca6..16a3a61d25326 100644
> --- a/drivers/hwmon/max6621.c
> +++ b/drivers/hwmon/max6621.c
> @@ -17,8 +17,8 @@
>  
>  #define MAX6621_DRV_NAME		"max6621"
>  #define MAX6621_TEMP_INPUT_REG_NUM	9
> -#define MAX6621_TEMP_INPUT_MIN		-127000
> -#define MAX6621_TEMP_INPUT_MAX		128000
> +#define MAX6621_TEMP_INPUT_MIN		-128000
> +#define MAX6621_TEMP_INPUT_MAX		127000

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
max6621_read() when handling hwmon_temp_crit_alarm:

drivers/hwmon/max6621.c:max6621_read() {
    ...
		case hwmon_temp_crit_alarm:
			...
			ret = regmap_read(data->regmap,
					  MAX6621_TEMP_ALERT_CAUSE_REG,
					  &regval);
			...
			if (regval) {
				ret = i2c_smbus_write_byte(data->client,
						MAX6621_CLEAR_ALERT_REG);
				if (ret)
					return ret;
			}

			*val = !!regval;
    ...
}

Because MAX6621_TEMP_ALERT_CAUSE_REG is global, does reading a single
channel's alarm attribute incorrectly return the global alert status
regardless of the requested channel?

Furthermore, could the subsequent write to MAX6621_CLEAR_ALERT_REG silently
clear the alert state for all other channels?

Since unprivileged users can typically read these alarm attributes, would
this allow a local user to suppress system-wide thermal alarms by
continuously reading a single channel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/9d3a4f1895a47794bb359a2a32fb1ccd6a15812c.1786334038.git.congnt264@gmail.com?part=1