Re: [PATCH v5 4/4] hwmon: it87: add IT8613E support

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The IT8613E configuration specifies support for 6 temperature sensors (num_temp_limit = 6), but omits the FEAT_SIX_TEMP feature flag, silently disabling sensors 4-6.

Pre-existing issues:
- [High] set_temp_type and get_temp_type corrupt and misinterpret IT87_REG_TEMP_ENABLE for temperature sensors 4, 5, and 6 due to incorrect bit-shifts exceeding the intended sensor bounds (0-2).
--

commit 62e828013c359cc9aa023fe24c4b5b0736549b6b
Author: benoit.masson <yahoo@perenite.com>

hwmon: it87: add IT8613E support
    
Teach the Super I/O probe path to recognize IT8613E and add its complete
hardware monitoring configuration. Add feature flags, temperature resource
counts, 11 mV ADC scaling, the IT8665-style PWM register map, and GPIO
pin-mux checks. Document the chip in the hwmon guide.

> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index e44ec65c76f03..67a94761d1c89 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
[ ... ]
> @@ -549,6 +552,19 @@ static const struct it87_devices it87_devices[] = {
>  		.num_temp_map = 4,
>  		.peci_mask = 0x07,
>  	},
> +	[it8613] = {
> +		.name = "it8613",
> +		.model = "IT8613E",
> +		.features = FEAT_NEWER_AUTOPWM | FEAT_11MV_ADC | FEAT_16BIT_FANS
> +		  | FEAT_TEMP_PECI | FEAT_FIVE_FANS
> +		  | FEAT_FIVE_PWM | FEAT_IN7_INTERNAL | FEAT_PWM_FREQ2
> +		  | FEAT_AVCC3 | FEAT_NEW_TEMPMAP,

[Severity: Medium]
Does this missing feature flag permanently disable sensors 4-6?

The IT8613E configuration specifies support for 6 temperature sensors with
num_temp_limit = 6, but the FEAT_SIX_TEMP feature flag is omitted here.
Without FEAT_SIX_TEMP, the probe logic in it87_probe() will not enable
the extra temperature sensors, leaving sensors 4-6 completely inaccessible
in sysfs:

drivers/hwmon/it87.c:it87_probe() {
    ...
	} else if (has_six_temp(data)) {
		u8 reg = it87_read_value(data, IT87_REG_TEMP456_ENABLE);

		/* Check for additional temperature sensors */
		if ((reg & 0x03) >= 0x02)
			data->has_temp |= BIT(3);
    ...
}

> +		.reg_pwm = IT87_REG_PWM_8665,
> +		.num_temp_limit = 6,

[Severity: High]
This is a pre-existing issue, but configuring 6 temperature sensors exposes
an out-of-bounds bitwise shift bug in get_temp_type() and set_temp_type().

When querying or setting types for sensors 4-6 (indices 3-5), get_temp_type()
misinterprets the configuration by reading bits 3-8 of IT87_REG_TEMP_ENABLE,
which belong to sensors 1-3:

drivers/hwmon/it87.c:get_temp_type() {
    ...
	else if (reg & BIT(index))
		type = 3;	/* thermal diode */
	else if (reg & BIT(index + 3))
		type = 4;	/* thermistor */
    ...
}

If those attributes become visible, set_temp_type() will apply out-of-bounds
bitwise shifts to IT87_REG_TEMP_ENABLE:

drivers/hwmon/it87.c:set_temp_type() {
    ...
	reg = it87_read_value(data, IT87_REG_TEMP_ENABLE);
	reg &= ~(1 << nr);
	reg &= ~(8 << nr);
	...
	if (val == 3)
		reg |= 1 << nr;
	else if (val == 4)
		reg |= 8 << nr;
    ...
}

Can this corrupt the chip configuration? Using unbounded nr indices (3-5)
will overwrite unrelated sensor configuration bits instead of targeting the
correct register, severely breaking thermal monitoring configuration for the
primary sensors and potentially leading to dangerous misbehavior in automated
fan control.

> +		.num_temp_offset = 6,
> +		.num_temp_map = 6,
> +		.peci_mask = 0x07,
> +	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829170545.33401-1-jerome.tollet@gmail.com?part=4