Skip to contentSkip to conversation

hwmon: using wrong ARRAY_SIZE() limit

PATCH In review Dan Carpenter 3 participants 4 messages Jul 17, 2013 – Jul 18, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hwmon: using wrong ARRAY_SIZE() limit

We cleaned up this code to use ARRAY_SIZE() instead of just the number
4. The problem is that data->reg_temp[] has 5 elements and we actually wanted ARRAY_SIZE(data->temp) which has 4 elements.

drivers/hwmon/nct6775.c +2 -2
index f0941d7..52b6a92 100644
@@ -1451,7 +1451,7 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 		for (i = 0; i < NUM_TEMP; i++) {
 			if (!(data->have_temp & (1 << i)))
 				continue;
-			for (j = 0; j < ARRAY_SIZE(data->reg_temp); j++) {
+			for (j = 0; j < ARRAY_SIZE(data->temp); j++) {
 				if (data->reg_temp[j][i])
 					data->temp[j][i]
 					  = nct6775_read_temp(data,
@@ -3974,7 +3974,7 @@ static int nct6775_resume(struct device *dev)
 		if (!(data->have_temp & (1 << i)))
 			continue;
 
-		for (j = 1; j < ARRAY_SIZE(data->reg_temp); j++)
+		for (j = 1; j < ARRAY_SIZE(data->temp); j++)
 			if (data->reg_temp[j][i])
 				nct6775_write_temp(data, data->reg_temp[j][i],
 						   data->temp[j][i]);
maintainer
#1 lore raw
Quoted 6 lines · Dan Carpenter

We cleaned up this code to use ARRAY_SIZE() instead of just the number
4. The problem is that data->reg_temp[] has 5 elements and we actually wanted ARRAY_SIZE(data->temp) which has 4 elements.

Hi Dan,

good catch, except that the array size of data->temp[] should be 5, not 4. So the real culprit is commit 7cbbd6a (Add support for critical low/high temperature limits on NCT6106) which increased the size of reg_temp but not the size of temp.

Do you want to send me a patch fixing that, or should I take care of it ?

Thanks,
Guenter

Quoted 23 lines
drivers/hwmon/nct6775.c +2 -2
index f0941d7..52b6a92 100644
@@ -1451,7 +1451,7 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 		for (i = 0; i < NUM_TEMP; i++) {
 			if (!(data->have_temp & (1 << i)))
 				continue;
-			for (j = 0; j < ARRAY_SIZE(data->reg_temp); j++) {
+			for (j = 0; j < ARRAY_SIZE(data->temp); j++) {
 				if (data->reg_temp[j][i])
 					data->temp[j][i]
 					  = nct6775_read_temp(data,
@@ -3974,7 +3974,7 @@ static int nct6775_resume(struct device *dev)
 		if (!(data->have_temp & (1 << i)))
 			continue;
 
-		for (j = 1; j < ARRAY_SIZE(data->reg_temp); j++)
+		for (j = 1; j < ARRAY_SIZE(data->temp); j++)
 			if (data->reg_temp[j][i])
 				nct6775_write_temp(data, data->reg_temp[j][i],
 						   data->temp[j][i]);
#2 lore raw
In reply to Dan Carpenter

Am 18.07.2013 01:36, schrieb Guenter Roeck:

Quoted 14 lines · Dan Carpenter
Quoted 6 lines

We cleaned up this code to use ARRAY_SIZE() instead of just the number
4. The problem is that data->reg_temp[] has 5 elements and we actually wanted ARRAY_SIZE(data->temp) which has 4 elements.

Hi Dan,

good catch, except that the array size of data->temp[] should be 5, not 4. So the real culprit is commit 7cbbd6a (Add support for critical low/high temperature limits on NCT6106) which increased the size of reg_temp but not the size of temp.

Quoted 4 lines

Do you want to send me a patch fixing that, or should I take care of it ?

Thanks,
Guenter

hi Guenther,
is an array the way to go here ? To avoid that kind of problems they sould be connected some how. Is it possible to replace the array with a malloc() ?

re,
wh

maintainer
#3 lore raw
In reply to Dan Carpenter
Quoted 17 lines · walter harms

Am 18.07.2013 01:36, schrieb Guenter Roeck:

Quoted 14 lines · Dan Carpenter
Quoted 6 lines

We cleaned up this code to use ARRAY_SIZE() instead of just the number
4. The problem is that data->reg_temp[] has 5 elements and we actually wanted ARRAY_SIZE(data->temp) which has 4 elements.

Hi Dan,

good catch, except that the array size of data->temp[] should be 5, not 4. So the real culprit is commit 7cbbd6a (Add support for critical low/high temperature limits on NCT6106) which increased the size of reg_temp but not the size of temp.

Quoted 9 lines
Quoted 4 lines

Do you want to send me a patch fixing that, or should I take care of it ?

Thanks,
Guenter

hi Guenther,
is an array the way to go here ? To avoid that kind of problems they sould be connected some how. Is it possible to replace the array with a malloc() ?

To clean it up I should probably add a define for the array sizes. A malloc would be overkill.

Guenter