nct6694-hwmon: temp17_enable and above write into the fan enable bitmap
Ali Ahmet Memis <[email protected]> Sun, 2 Aug 2026 12:47:30 +0000
| Newsgroups | org.kernel.vger.linux-hwmon,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi,
While reading nct6694-hwmon.c I ran into something that looks wrong, but
which way to fix it depends on the device protocol, so I would rather ask
than guess.
The driver registers 26 temperature channels, each with HWMON_T_ENABLE:
HWMON_CHANNEL_INFO(temp,
NCT6694_HWMON_TEMP_CONFIG, /* THR1 */
...
NCT6694_HWMON_TEMP_CONFIG), /* DTIN15 */
That is 5 THR plus 5 TDP plus 16 DTIN. The enable bitmap it indexes is
two bytes:
struct __packed nct6694_hwmon_control {
u8 vin_en[2];
u8 tin_en[2];
u8 fin_en[2];
u8 pwm_en[2];
u8 reserved1[40];
u8 pwm_freq[10];
u8 reserved2[6];
};
and both the read and the write path index it with the raw channel:
temp_en = data->hwmon_en.tin_en[channel / 8];
...
data->hwmon_en.tin_en[channel / 8] |= BIT(channel % 8);
nct6694_is_visible() returns 0644 for hwmon_temp_enable without looking
at the channel number, so all 26 are writable. Since the structure is
packed, channel 16 and above land past tin_en[]:
channel 0-15 -> tin_en[0-1]
channel 16-23 -> tin_en[2] == fin_en[0]
channel 24-25 -> tin_en[3] == fin_en[1]
nct6694_hwmon_init() sends the whole control structure back to the
device, so writing temp17_enable through temp26_enable ends up toggling
the fan enable bits, and reading them reports fan state as temperature
state. It stays inside the structure, so this is not a memory safety
problem, but on a board using the fan channels it is not harmless.
What I cannot tell from here is which side is wrong, and the answers
need different patches:
1. If the control block really has two bytes of tin_en, then only 16
temperature channels can be enabled and the last ten should not
advertise HWMON_T_ENABLE.
2. If the device has four bytes of tin_en, the structure is wrong, and
fin_en and pwm_en are being read and written at the wrong offsets as
well.
3. If the bitmap only covers the digital inputs, the indexing is wrong
in a third way: there are exactly 16 DTIN channels, which is exactly
what tin_en[2] holds, and they sit at channel 10 and above.
The structure is 64 bytes as declared, and it would still be 64 bytes
with tin_en[4] and reserved1[38], so the total size does not settle it.
Two things make me unsure rather than confident: the other three bitmaps
match their channel counts exactly, 16 in channels against vin_en[2] and
10 fan channels against fin_en[2], which argues for the first case; but
the 16 DTIN channels also match tin_en[2] exactly, which is what made me
think of the third. I could not find a public datasheet to settle it.
Ming, could you say which one it is? I am happy to write the patch once
I know, I just do not want to guess at the device side.
I have no NCT6694 hardware, so this is from reading the driver against
current mainline rather than from an observed misbehaviour.
Thanks,
Ali