* [PATCH v2] lm85: Bounds check to_sensor_dev_attr()->index usage
@ 2023-02-03 22:32 Kees Cook
0 siblings, 0 replies; only message in thread
From: Kees Cook @ 2023-02-03 22:32 UTC (permalink / raw)
To: Jean Delvare
Cc: Kees Cook, Guenter Roeck, linux-hwmon, linux-kernel, linux-hardening
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 --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;
};
--
2.34.1
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2023-02-03 22:33 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03 22:32 [PATCH v2] lm85: Bounds check to_sensor_dev_attr()->index usage Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.