hwmon: (nct6694) do not expose enable on DTIN temperature channels

[PATCH] hwmon: (nct6694) do not expose enable on DTIN temperature channels

Posted by Ali Ahmet Memis 1 month, 4 weeks ago

The driver registers 26 temperature channels, all advertising
HWMON_T_ENABLE, and indexes the enable bitmap with the raw channel:

	data->hwmon_en.tin_en[channel / 8] |= BIT(channel % 8);

tin_en is two bytes and only covers the 5 THR and 5 TDP channels
(index 0-9). The 16 DTIN channels (index 10-25) are enabled by the
firmware and were never meant to carry an enable bit. Because the
control structure is packed, writing temp17_enable and above indexes
past tin_en into the fin_en bytes that follow it, so it toggles fan
enable state instead; nct6694_hwmon_init() then sends the whole
structure back to the device, and reads report fan state as temperature
state. It stays within the structure, so this is not a memory safety
problem, but on a board that uses the fan channels it is not harmless.

Give the DTIN channels a temperature config without HWMON_T_ENABLE so
the core never creates their enable attribute. The enable path is then
reachable only for the first 10 channels, which stay within tin_en, and
fin_en is left alone. The DTIN input and limit attributes are unchanged.

Fixes: 197e779d29d8 ("hwmon: Add Nuvoton NCT6694 HWMON support")
Suggested-by: Ming Yu <tmyu0@nuvoton.com>
Link: https://lore.kernel.org/all/20260802124730.20387-1-ali@iusegentoo.com/
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 drivers/hwmon/nct6694-hwmon.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/hwmon/nct6694-hwmon.c b/drivers/hwmon/nct6694-hwmon.c
index 6dcf22ca5018..9a9a4db434c4 100644
--- a/drivers/hwmon/nct6694-hwmon.c
+++ b/drivers/hwmon/nct6694-hwmon.c
@@ -159,6 +159,9 @@ static inline s8 temp_to_reg(long val)
 #define NCT6694_HWMON_TEMP_CONFIG (HWMON_T_INPUT | HWMON_T_ENABLE |	\
 				   HWMON_T_MAX | HWMON_T_MAX_HYST |	\
 				   HWMON_T_MAX_ALARM)
+#define NCT6694_HWMON_DTIN_CONFIG (HWMON_T_INPUT |			\
+				   HWMON_T_MAX | HWMON_T_MAX_HYST |	\
+				   HWMON_T_MAX_ALARM)
 #define NCT6694_HWMON_FAN_CONFIG (HWMON_F_INPUT | HWMON_F_ENABLE |	\
 				  HWMON_F_MIN | HWMON_F_MIN_ALARM)
 #define NCT6694_HWMON_PWM_CONFIG (HWMON_PWM_INPUT | HWMON_PWM_ENABLE |	\
@@ -193,22 +196,22 @@ static const struct hwmon_channel_info *nct6694_info[] = {
 			   NCT6694_HWMON_TEMP_CONFIG,	/* TDP2 */
 			   NCT6694_HWMON_TEMP_CONFIG,	/* TDP3 */
 			   NCT6694_HWMON_TEMP_CONFIG,	/* TDP4 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN0 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN1 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN2 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN3 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN4 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN5 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN6 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN7 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN8 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN9 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN10 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN11 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN12 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN13 */
-			   NCT6694_HWMON_TEMP_CONFIG,	/* DTIN14 */
-			   NCT6694_HWMON_TEMP_CONFIG),	/* DTIN15 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN0 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN1 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN2 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN3 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN4 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN5 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN6 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN7 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN8 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN9 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN10 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN11 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN12 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN13 */
+			   NCT6694_HWMON_DTIN_CONFIG,	/* DTIN14 */
+			   NCT6694_HWMON_DTIN_CONFIG),	/* DTIN15 */
 
 	HWMON_CHANNEL_INFO(fan,
 			   NCT6694_HWMON_FAN_CONFIG,	/* FIN0 */
-- 
2.55.0

Re: [PATCH] hwmon: (nct6694) do not expose enable on DTIN temperature channels

Posted by Guenter Roeck 3 weeks, 1 day ago

On Mon, Aug 03, 2026 at 10:21:48AM +0000, Ali Ahmet Memis wrote:
> The driver registers 26 temperature channels, all advertising
> HWMON_T_ENABLE, and indexes the enable bitmap with the raw channel:
> 
> 	data->hwmon_en.tin_en[channel / 8] |= BIT(channel % 8);
> 
> tin_en is two bytes and only covers the 5 THR and 5 TDP channels
> (index 0-9). The 16 DTIN channels (index 10-25) are enabled by the
> firmware and were never meant to carry an enable bit. Because the
> control structure is packed, writing temp17_enable and above indexes
> past tin_en into the fin_en bytes that follow it, so it toggles fan
> enable state instead; nct6694_hwmon_init() then sends the whole
> structure back to the device, and reads report fan state as temperature
> state. It stays within the structure, so this is not a memory safety
> problem, but on a board that uses the fan channels it is not harmless.
> 
> Give the DTIN channels a temperature config without HWMON_T_ENABLE so
> the core never creates their enable attribute. The enable path is then
> reachable only for the first 10 channels, which stay within tin_en, and
> fin_en is left alone. The DTIN input and limit attributes are unchanged.
> 
> Fixes: 197e779d29d8 ("hwmon: Add Nuvoton NCT6694 HWMON support")
> Suggested-by: Ming Yu <tmyu0@nuvoton.com>
> Link: https://lore.kernel.org/all/20260802124730.20387-1-ali@iusegentoo.com/
> Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>

Applied.

Thanks,
Guenter