Re: [RFC PATCH] hwmon: (nct6775) Ignore 0xffff TSI temperature readings

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `tsi_temp` array is incorrectly declared as `s16` (signed) instead of `u16`. As a result, when an active TSI channel returns `0xffff` dynamically at runtime, the value is stored as a negative number (`-1`). When passed to `show_tsi_temp()` and then to `tsi_temp_from_reg(unsigned int reg)`, it is sign-extended to a 32-bit integer (`0xffffffff`). The function then performs a logical right shift (`reg >> 5`), corrupting the value and yielding a bogus reading of 3892313987 millidegrees Celsius. While the patch hides `0xffff` at probe time, it leaves the root cause (the `s16` array type) unfixed, allowing the bug to trigger during runtime updates.
--

--- Patch [1]: [RFC PATCH] hwmon: (nct6775) Ignore 0xffff TSI temperature readings ---
commit 4af5e21bfe7d5aa83ca756cf26ad0f2043522f36
Author: Spector Tsai <Spector.Tsai@outlook.com>

hwmon: (nct6775) Ignore 0xffff TSI temperature readings

This patch updates the probe path to treat 0xffff as an inactive TSI channel.
It aims to prevent unused TSI channels from being exposed and reporting bogus
temperature readings of 3892313987 millidegrees Celsius.

> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
> index 759ea3d1286e3..34e6a01aa4e49 100644
> --- a/drivers/hwmon/nct6775-core.c
> +++ b/drivers/hwmon/nct6775-core.c
> @@ -4339,7 +4339,7 @@ int nct6775_probe(struct device *dev, struct nct6775_data *data,
>  		err = nct6775_read_value(data, data->REG_TSI_TEMP[i], &tmp);
>  		if (err)
>  			return err;
> -		if (tmp)
> +		if (tmp && tmp != 0xffff)

[Severity: High]
This isn't a bug introduced by this patch, but does this fully resolve the
bogus reading issue if an active channel returns 0xffff dynamically at
runtime?

Looking at drivers/hwmon/nct6775.h, the tsi_temp array is declared as s16:

struct nct6775_data {
    ...
    s16 tsi_temp[NUM_TSI_TEMP];
    ...
}

In nct6775_update_device(), when reading a TSI channel that temporarily
returns 0xffff, this 16-bit value is stored into the s16 array as -1.

When the temperature is exported to sysfs:

drivers/hwmon/nct6775-core.c:show_tsi_temp() {
    ...
    return sysfs_emit(buf, "%u\n",
                      tsi_temp_from_reg(data->tsi_temp[sattr->index]));
}

The s16 value is passed to tsi_temp_from_reg() which takes an unsigned int:

drivers/hwmon/nct6775-core.c:tsi_temp_from_reg() {
    return (reg >> 5) * 125;
}

The -1 is implicitly cast to an unsigned int (0xffffffff), and the logical
right shift yields the 3892313987 millidegrees Celsius bogus reading. If
userspace thermal managers observe this massive value during runtime updates,
could it trigger an unwarranted emergency system shutdown?

Would changing the tsi_temp array type to u16 prevent this sign-extension
issue entirely?

>  			data->have_tsi_temp |= BIT(i);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824184443.81887-1-Spector.Tsai@outlook.com?part=1