diff mbox series

[v2] lm85: Bounds check to_sensor_dev_attr()->index usage

Message ID 20230203223250.gonna.713-kees@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2] lm85: Bounds check to_sensor_dev_attr()->index usage | expand

Commit Message

Kees Cook Feb. 3, 2023, 10:32 p.m. UTC 
The index into various register arrays was not bounds checked. Provide a
simple wrapper to bounds check the index, adding robustness in the face
of memory corruption, unexpected index manipulation, etc. Noticed under
GCC 13 with -Warray-bounds:

drivers/hwmon/lm85.c: In function 'pwm_auto_pwm_minctl_store':
drivers/hwmon/lm85.c:1110:26: warning: array subscript [0, 31] is outside array bounds of 'struct lm85_autofan[3]' [-Warray-bounds=]
 1110 |         if (data->autofan[nr].min_off)
      |             ~~~~~~~~~~~~~^~~~
drivers/hwmon/lm85.c:317:29: note: while referencing 'autofan'
  317 |         struct lm85_autofan autofan[3];
      |                             ^~~~~~~

Additionally fix a comment indentation and a coding style issue of
a missing space before the { in the struct sensor_device_attribute
definition.

Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2: create a common helper for bounds checking
v1: https://lore.kernel.org/linux-hardening/20230127223744.never.113-kees@kernel.org/
---
 drivers/hwmon/lm85.c        | 77 +++++++++++++++++++++----------------
 include/linux/hwmon-sysfs.h |  2 +-
 2 files changed, 44 insertions(+), 35 deletions(-)
diff mbox series

Patch

diff --git a/drivers/hwmon/lm85.c b/drivers/hwmon/lm85.c
index 8d33c2484755..3cb04e4138b8 100644
--- a/drivers/hwmon/lm85.c
+++ b/drivers/hwmon/lm85.c
@@ -318,6 +318,15 @@  struct lm85_data {
 	struct lm85_zone zone[3];
 };
 
+#define get_sensor_index(attr, array)				\
+({								\
+	unsigned int _nr = to_sensor_dev_attr(attr)->index;	\
+								\
+	if (WARN_ON_ONCE(_nr >= ARRAY_SIZE(array)))		\
+		_nr = 0;					\
+	_nr;							\
+})
+
 static int lm85_read_value(struct i2c_client *client, u8 reg)
 {
 	int res;
@@ -552,16 +561,16 @@  static struct lm85_data *lm85_update_device(struct device *dev)
 static ssize_t fan_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->fan);
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[nr]));
 }
 
 static ssize_t fan_min_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->fan_min);
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[nr]));
 }
 
@@ -569,8 +578,8 @@  static ssize_t fan_min_store(struct device *dev,
 			     struct device_attribute *attr, const char *buf,
 			     size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->fan_min);
 	struct i2c_client *client = data->client;
 	unsigned long val;
 	int err;
@@ -683,16 +692,16 @@  static SENSOR_DEVICE_ATTR_RO(fan4_alarm, alarm, 13);
 static ssize_t pwm_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->pwm);
 	return sprintf(buf, "%d\n", PWM_FROM_REG(data->pwm[nr]));
 }
 
 static ssize_t pwm_store(struct device *dev, struct device_attribute *attr,
 			 const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->pwm);
 	struct i2c_client *client = data->client;
 	unsigned long val;
 	int err;
@@ -711,8 +720,8 @@  static ssize_t pwm_store(struct device *dev, struct device_attribute *attr,
 static ssize_t pwm_enable_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	int pwm_zone, enable;
 
 	pwm_zone = ZONE_FROM_REG(data->autofan[nr].config);
@@ -734,8 +743,8 @@  static ssize_t pwm_enable_store(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	struct i2c_client *client = data->client;
 	u8 config;
 	unsigned long val;
@@ -777,8 +786,8 @@  static ssize_t pwm_enable_store(struct device *dev,
 static ssize_t pwm_freq_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->pwm_freq);
 	int freq;
 
 	if (IS_ADT7468_HFPWM(data))
@@ -794,8 +803,8 @@  static ssize_t pwm_freq_store(struct device *dev,
 			      struct device_attribute *attr, const char *buf,
 			      size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->pwm_freq);
 	struct i2c_client *client = data->client;
 	unsigned long val;
 	int err;
@@ -843,8 +852,8 @@  static SENSOR_DEVICE_ATTR_RW(pwm3_freq, pwm_freq, 2);
 static ssize_t in_show(struct device *dev, struct device_attribute *attr,
 		       char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->in);
 	return sprintf(buf, "%d\n", INSEXT_FROM_REG(nr, data->in[nr],
 						    data->in_ext[nr]));
 }
@@ -852,16 +861,16 @@  static ssize_t in_show(struct device *dev, struct device_attribute *attr,
 static ssize_t in_min_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->in);
 	return sprintf(buf, "%d\n", INS_FROM_REG(nr, data->in_min[nr]));
 }
 
 static ssize_t in_min_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->in_min);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -880,16 +889,16 @@  static ssize_t in_min_store(struct device *dev, struct device_attribute *attr,
 static ssize_t in_max_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->in_max);
 	return sprintf(buf, "%d\n", INS_FROM_REG(nr, data->in_max[nr]));
 }
 
 static ssize_t in_max_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->in_max);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -935,8 +944,8 @@  static SENSOR_DEVICE_ATTR_RW(in7_max, in_max, 7);
 static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->temp);
 	return sprintf(buf, "%d\n", TEMPEXT_FROM_REG(data->temp[nr],
 						     data->temp_ext[nr]));
 }
@@ -944,8 +953,8 @@  static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
 static ssize_t temp_min_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->temp_min);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_min[nr]));
 }
 
@@ -953,8 +962,8 @@  static ssize_t temp_min_store(struct device *dev,
 			      struct device_attribute *attr, const char *buf,
 			      size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->temp_min);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -976,8 +985,8 @@  static ssize_t temp_min_store(struct device *dev,
 static ssize_t temp_max_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->temp_max);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_max[nr]));
 }
 
@@ -985,8 +994,8 @@  static ssize_t temp_max_store(struct device *dev,
 			      struct device_attribute *attr, const char *buf,
 			      size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->temp_max);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -1021,8 +1030,8 @@  static ssize_t pwm_auto_channels_show(struct device *dev,
 				      struct device_attribute *attr,
 				      char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	return sprintf(buf, "%d\n", ZONE_FROM_REG(data->autofan[nr].config));
 }
 
@@ -1030,8 +1039,8 @@  static ssize_t pwm_auto_channels_store(struct device *dev,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -1052,8 +1061,8 @@  static ssize_t pwm_auto_channels_store(struct device *dev,
 static ssize_t pwm_auto_pwm_min_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	return sprintf(buf, "%d\n", PWM_FROM_REG(data->autofan[nr].min_pwm));
 }
 
@@ -1061,8 +1070,8 @@  static ssize_t pwm_auto_pwm_min_store(struct device *dev,
 				      struct device_attribute *attr,
 				      const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	struct i2c_client *client = data->client;
 	unsigned long val;
 	int err;
@@ -1083,8 +1092,8 @@  static ssize_t pwm_auto_pwm_minctl_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	return sprintf(buf, "%d\n", data->autofan[nr].min_off);
 }
 
@@ -1092,8 +1101,8 @@  static ssize_t pwm_auto_pwm_minctl_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->autofan);
 	struct i2c_client *client = data->client;
 	u8 tmp;
 	long val;
@@ -1130,8 +1139,8 @@  static ssize_t temp_auto_temp_off_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit) -
 		HYST_FROM_REG(data->zone[nr].hyst));
 }
@@ -1140,8 +1149,8 @@  static ssize_t temp_auto_temp_off_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	struct i2c_client *client = data->client;
 	int min;
 	long val;
@@ -1170,8 +1179,8 @@  static ssize_t temp_auto_temp_min_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit));
 }
 
@@ -1179,8 +1188,8 @@  static ssize_t temp_auto_temp_min_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
@@ -1194,7 +1203,7 @@  static ssize_t temp_auto_temp_min_store(struct device *dev,
 	lm85_write_value(client, LM85_REG_AFAN_LIMIT(nr),
 		data->zone[nr].limit);
 
-/* Update temp_auto_max and temp_auto_range */
+	/* Update temp_auto_max and temp_auto_range */
 	data->zone[nr].range = RANGE_TO_REG(
 		TEMP_FROM_REG(data->zone[nr].max_desired) -
 		TEMP_FROM_REG(data->zone[nr].limit));
@@ -1210,8 +1219,8 @@  static ssize_t temp_auto_temp_max_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit) +
 		RANGE_FROM_REG(data->zone[nr].range));
 }
@@ -1220,8 +1229,8 @@  static ssize_t temp_auto_temp_max_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	struct i2c_client *client = data->client;
 	int min;
 	long val;
@@ -1247,8 +1256,8 @@  static ssize_t temp_auto_temp_crit_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = lm85_update_device(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].critical));
 }
 
@@ -1256,8 +1265,8 @@  static ssize_t temp_auto_temp_crit_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t count)
 {
-	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
+	unsigned int nr = get_sensor_index(attr, data->zone);
 	struct i2c_client *client = data->client;
 	long val;
 	int err;
diff --git a/include/linux/hwmon-sysfs.h b/include/linux/hwmon-sysfs.h
index d896713359cd..f0505d10bfad 100644
--- a/include/linux/hwmon-sysfs.h
+++ b/include/linux/hwmon-sysfs.h
@@ -10,7 +10,7 @@ 
 #include <linux/device.h>
 #include <linux/kstrtox.h>
 
-struct sensor_device_attribute{
+struct sensor_device_attribute {
 	struct device_attribute dev_attr;
 	int index;
 };