diff mbox series

nct6775: Fix access to temperature configuration registers

Message ID 20240221141345.2231350-1-linux@roeck-us.net (mailing list archive)
State Accepted
Headers show
Series nct6775: Fix access to temperature configuration registers | expand

Commit Message

Guenter Roeck Feb. 21, 2024, 2:13 p.m. UTC
The number of temperature configuration registers does
not always match the total number of temperature registers.
This can result in access errors reported if KASAN is enabled.

BUG: KASAN: global-out-of-bounds in nct6775_probe+0x5654/0x6fe9 nct6775_core

Reported-by: Erhard Furtner <erhard_f@mailbox.org>
Closes: https://lore.kernel.org/linux-hwmon/d51181d1-d26b-42b2-b002-3f5a4037721f@roeck-us.net/
Fixes: 578ab5f0e4b1 ("hwmon: (nct6775) Add support for NCT6791D")
Fixes: b7f1f7b2523a ("hwmon: (nct6775) Additional TEMP registers for nct6799")
Cc: Ahmad Khalifa <ahmad@khalifa.ws>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Erhard,

it would be great if you can test this patch on your system.

Thanks,
Guenter

 drivers/hwmon/nct6775-core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Ahmad Khalifa Feb. 21, 2024, 9:31 p.m. UTC | #1
On 21/02/2024 14:13, Guenter Roeck wrote:
> The number of temperature configuration registers does
> not always match the total number of temperature registers.
> This can result in access errors reported if KASAN is enabled.
> 
> BUG: KASAN: global-out-of-bounds in nct6775_probe+0x5654/0x6fe9 nct6775_core
> 
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Closes: https://lore.kernel.org/linux-hwmon/d51181d1-d26b-42b2-b002-3f5a4037721f@roeck-us.net/
> Fixes: 578ab5f0e4b1 ("hwmon: (nct6775) Add support for NCT6791D")
> Fixes: b7f1f7b2523a ("hwmon: (nct6775) Additional TEMP registers for nct6799")
> Cc: Ahmad Khalifa <ahmad@khalifa.ws>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

Tested-by: Ahmad Khalifa <ahmad@khalifa.ws>


Tested on nct6799D-R without KASAN - nothing to report.

Tiny note, i is in the range of 0..7 on nct6798/nct6799 if I
follow correctly? Still 8 > 2, well caught.
Guenter Roeck Feb. 21, 2024, 9:59 p.m. UTC | #2
Hi,

On 2/21/24 13:31, Ahmad Khalifa wrote:
> On 21/02/2024 14:13, Guenter Roeck wrote:
>> The number of temperature configuration registers does
>> not always match the total number of temperature registers.
>> This can result in access errors reported if KASAN is enabled.
>>
>> BUG: KASAN: global-out-of-bounds in nct6775_probe+0x5654/0x6fe9 nct6775_core
>>
>> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
>> Closes: https://lore.kernel.org/linux-hwmon/d51181d1-d26b-42b2-b002-3f5a4037721f@roeck-us.net/
>> Fixes: 578ab5f0e4b1 ("hwmon: (nct6775) Add support for NCT6791D")
>> Fixes: b7f1f7b2523a ("hwmon: (nct6775) Additional TEMP registers for nct6799")
>> Cc: Ahmad Khalifa <ahmad@khalifa.ws>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
> 
> Tested-by: Ahmad Khalifa <ahmad@khalifa.ws>
> 
> 
> Tested on nct6799D-R without KASAN - nothing to report.
> 
Thanks for testing. Note that you would only see the problem if KASAN is enabled
(otherwise the code just sets a more or less random register address). On top of
that it depends on the chip configuration.

> Tiny note, i is in the range of 0..7 on nct6798/nct6799 if I
> follow correctly? Still 8 > 2, well caught.
> 
Sorry, I don't understand your comment. Yes, i is in the range of 0..7 on
nct6798/nct6799, which triggers the failure if i >= 2 because the code uses
the NCT6779_REG_TEMP_CONFIG array to identify configuration registers,
and that array only has two entries.

Thanks,
Guenter
Ahmad Khalifa Feb. 21, 2024, 10:12 p.m. UTC | #3
On 21/02/2024 21:59, Guenter Roeck wrote:
>> Tiny note, i is in the range of 0..7 on nct6798/nct6799 if I
>> follow correctly? Still 8 > 2, well caught.
>>
> Sorry, I don't understand your comment. Yes, i is in the range of 0..7 on
> nct6798/nct6799, which triggers the failure if i >= 2 because the code uses
> the NCT6779_REG_TEMP_CONFIG array to identify configuration registers,
> and that array only has two entries.

Ok, I think I was following correctly, just thrown off by the comment
on the bug report thread about i being 0..11:
> The range of "i" is 0..11, and the size of the reg_temp_config[] array is 2. Oops.
Guenter Roeck Feb. 21, 2024, 10:22 p.m. UTC | #4
On 2/21/24 14:12, Ahmad Khalifa wrote:
> On 21/02/2024 21:59, Guenter Roeck wrote:
>>> Tiny note, i is in the range of 0..7 on nct6798/nct6799 if I
>>> follow correctly? Still 8 > 2, well caught.
>>>
>> Sorry, I don't understand your comment. Yes, i is in the range of 0..7 on
>> nct6798/nct6799, which triggers the failure if i >= 2 because the code uses
>> the NCT6779_REG_TEMP_CONFIG array to identify configuration registers,
>> and that array only has two entries.
> 
> Ok, I think I was following correctly, just thrown off by the comment
> on the bug report thread about i being 0..11:
>> The range of "i" is 0..11, and the size of the reg_temp_config[] array is 2. Oops.

Ah, ok. I should have said "the possible range..."

Guenter
Erhard Furtner Feb. 21, 2024, 10:53 p.m. UTC | #5
On Wed, 21 Feb 2024 06:13:45 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> The number of temperature configuration registers does
> not always match the total number of temperature registers.
> This can result in access errors reported if KASAN is enabled.
> 
> BUG: KASAN: global-out-of-bounds in nct6775_probe+0x5654/0x6fe9 nct6775_core
> 
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Closes: https://lore.kernel.org/linux-hwmon/d51181d1-d26b-42b2-b002-3f5a4037721f@roeck-us.net/
> Fixes: 578ab5f0e4b1 ("hwmon: (nct6775) Add support for NCT6791D")
> Fixes: b7f1f7b2523a ("hwmon: (nct6775) Additional TEMP registers for nct6799")
> Cc: Ahmad Khalifa <ahmad@khalifa.ws>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Erhard,
> 
> it would be great if you can test this patch on your system.
> 
> Thanks,
> Guenter

Applied your patch on top of 6.8-rc5 and 6.6.0.

Works fine in both cases, the KASAN hit is gone. Many thanks!

Regards,
Erhard
Guenter Roeck Feb. 21, 2024, 11:39 p.m. UTC | #6
Hi Erhard,

On 2/21/24 14:53, Erhard Furtner wrote:
> On Wed, 21 Feb 2024 06:13:45 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> The number of temperature configuration registers does
>> not always match the total number of temperature registers.
>> This can result in access errors reported if KASAN is enabled.
>>
>> BUG: KASAN: global-out-of-bounds in nct6775_probe+0x5654/0x6fe9 nct6775_core
>>
>> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
>> Closes: https://lore.kernel.org/linux-hwmon/d51181d1-d26b-42b2-b002-3f5a4037721f@roeck-us.net/
>> Fixes: 578ab5f0e4b1 ("hwmon: (nct6775) Add support for NCT6791D")
>> Fixes: b7f1f7b2523a ("hwmon: (nct6775) Additional TEMP registers for nct6799")
>> Cc: Ahmad Khalifa <ahmad@khalifa.ws>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> Erhard,
>>
>> it would be great if you can test this patch on your system.
>>
>> Thanks,
>> Guenter
> 
> Applied your patch on top of 6.8-rc5 and 6.6.0.
> 
> Works fine in both cases, the KASAN hit is gone. Many thanks!
> 

Excellent. Thanks a lot for confirming!

Guenter
diff mbox series

Patch

diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
index 8d2ef3145bca..9fbab8f02334 100644
--- a/drivers/hwmon/nct6775-core.c
+++ b/drivers/hwmon/nct6775-core.c
@@ -3512,6 +3512,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 	const u16 *reg_temp_mon, *reg_temp_alternate, *reg_temp_crit;
 	const u16 *reg_temp_crit_l = NULL, *reg_temp_crit_h = NULL;
 	int num_reg_temp, num_reg_temp_mon, num_reg_tsi_temp;
+	int num_reg_temp_config;
 	struct device *hwmon_dev;
 	struct sensor_template_group tsi_temp_tg;
 
@@ -3594,6 +3595,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6106_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6106_REG_TEMP_HYST;
 		reg_temp_config = NCT6106_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6106_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6106_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6106_REG_TEMP_CRIT;
 		reg_temp_crit_l = NCT6106_REG_TEMP_CRIT_L;
@@ -3669,6 +3671,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6106_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6106_REG_TEMP_HYST;
 		reg_temp_config = NCT6106_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6106_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6106_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6106_REG_TEMP_CRIT;
 		reg_temp_crit_l = NCT6106_REG_TEMP_CRIT_L;
@@ -3746,6 +3749,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6775_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6775_REG_TEMP_HYST;
 		reg_temp_config = NCT6775_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6775_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6775_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6775_REG_TEMP_CRIT;
 
@@ -3821,6 +3825,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6775_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6775_REG_TEMP_HYST;
 		reg_temp_config = NCT6776_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6776_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6776_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6776_REG_TEMP_CRIT;
 
@@ -3900,6 +3905,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6779_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6779_REG_TEMP_HYST;
 		reg_temp_config = NCT6779_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6779_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6779_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6779_REG_TEMP_CRIT;
 
@@ -4034,6 +4040,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6779_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6779_REG_TEMP_HYST;
 		reg_temp_config = NCT6779_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6779_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6779_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6779_REG_TEMP_CRIT;
 
@@ -4123,6 +4130,7 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		reg_temp_over = NCT6798_REG_TEMP_OVER;
 		reg_temp_hyst = NCT6798_REG_TEMP_HYST;
 		reg_temp_config = NCT6779_REG_TEMP_CONFIG;
+		num_reg_temp_config = ARRAY_SIZE(NCT6779_REG_TEMP_CONFIG);
 		reg_temp_alternate = NCT6798_REG_TEMP_ALTERNATE;
 		reg_temp_crit = NCT6798_REG_TEMP_CRIT;
 
@@ -4204,7 +4212,8 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 				  = reg_temp_crit[src - 1];
 			if (reg_temp_crit_l && reg_temp_crit_l[i])
 				data->reg_temp[4][src - 1] = reg_temp_crit_l[i];
-			data->reg_temp_config[src - 1] = reg_temp_config[i];
+			if (i < num_reg_temp_config)
+				data->reg_temp_config[src - 1] = reg_temp_config[i];
 			data->temp_src[src - 1] = src;
 			continue;
 		}
@@ -4217,7 +4226,8 @@  int nct6775_probe(struct device *dev, struct nct6775_data *data,
 		data->reg_temp[0][s] = reg_temp[i];
 		data->reg_temp[1][s] = reg_temp_over[i];
 		data->reg_temp[2][s] = reg_temp_hyst[i];
-		data->reg_temp_config[s] = reg_temp_config[i];
+		if (i < num_reg_temp_config)
+			data->reg_temp_config[s] = reg_temp_config[i];
 		if (reg_temp_crit_h && reg_temp_crit_h[i])
 			data->reg_temp[3][s] = reg_temp_crit_h[i];
 		else if (reg_temp_crit[src - 1])