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

From: Kees Cook
Date: Fri Jan 27 2023 - 17:37:56 EST

Next message: Paul Moore: "Re: [PATCH v1 0/2] two suggested iouring op audit updates"
Previous message: Jeff LaBundy: "Re: [PATCH] dt-bindings: input: azoteq: Fix differing types"
Next in thread: Guenter Roeck: "Re: [PATCH] lm85: Bounds check to_sensor_dev_attr()->index usage"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

The index into various register arrays was not bounds checked. Add
checking. Seen under GCC 13:

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];
| ^~~~~~~

Cc: Jean Delvare <jdelvare@xxxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
Cc: linux-hwmon@xxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
drivers/hwmon/lm85.c | 221 +++++++++++++++++++++++++++---------
include/linux/hwmon-sysfs.h | 2 +-
2 files changed, 167 insertions(+), 56 deletions(-)

diff --git a/drivers/hwmon/lm85.c b/drivers/hwmon/lm85.c
index 8d33c2484755..367a77660dda 100644
--- a/drivers/hwmon/lm85.c
+++ b/drivers/hwmon/lm85.c
@@ -552,29 +552,40 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->fan))
+ val = FAN_FROM_REG(data->fan[nr]);
+ return sprintf(buf, "%d\n", val);
}

static ssize_t fan_min_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- int nr = to_sensor_dev_attr(attr)->index;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->fan_min))
+ val = FAN_FROM_REG(data->fan_min[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
unsigned long val;
int err;

+ if (nr >= ARRAY_SIZE(data->fan_min))
+ return -EINVAL;
+
err = kstrtoul(buf, 10, &val);
if (err)
return err;
@@ -683,20 +694,27 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", PWM_FROM_REG(data->pwm[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->pwm))
+ val = PWM_FROM_REG(data->pwm[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
unsigned long val;
int err;

+ if (nr >= ARRAY_SIZE(data->pwm))
+ return -EINVAL;
+
err = kstrtoul(buf, 10, &val);
if (err)
return err;
@@ -711,11 +729,12 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- int pwm_zone, enable;
+ int pwm_zone = -1, enable;

- pwm_zone = ZONE_FROM_REG(data->autofan[nr].config);
+ if (nr < ARRAY_SIZE(data->autofan))
+ pwm_zone = ZONE_FROM_REG(data->autofan[nr].config);
switch (pwm_zone) {
case -1: /* PWM is always at 100% */
enable = 0;
@@ -734,13 +753,16 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
u8 config;
unsigned long val;
int err;

+ if (nr >= ARRAY_SIZE(data->autofan))
+ return -EINVAL;
+
err = kstrtoul(buf, 10, &val);
if (err)
return err;
@@ -777,10 +799,13 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
int freq;

+ if (nr >= ARRAY_SIZE(data->pwm_freq))
+ return -EINVAL;
+
if (IS_ADT7468_HFPWM(data))
freq = 22500;
else
@@ -794,12 +819,15 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
unsigned long val;
int err;

+ if (nr >= ARRAY_SIZE(data->pwm_freq))
+ return -EINVAL;
+
err = kstrtoul(buf, 10, &val);
if (err)
return err;
@@ -843,10 +871,13 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", INSEXT_FROM_REG(nr, data->in[nr],
- data->in_ext[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->in))
+ val = INSEXT_FROM_REG(nr, data->in[nr], data->in_ext[nr]);
+ return sprintf(buf, "%d\n", val);
}

static ssize_t in_min_show(struct device *dev, struct device_attribute *attr,
@@ -854,18 +885,25 @@ static ssize_t in_min_show(struct device *dev, struct device_attribute *attr,
{
int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", INS_FROM_REG(nr, data->in_min[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->in_min))
+ val = INS_FROM_REG(nr, data->in_min[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->in_min))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -880,20 +918,27 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", INS_FROM_REG(nr, data->in_max[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->in_max))
+ val = INS_FROM_REG(nr, data->in_max[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->in_max))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -935,30 +980,40 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMPEXT_FROM_REG(data->temp[nr],
- data->temp_ext[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->temp))
+ val = TEMPEXT_FROM_REG(data->temp[nr], data->temp_ext[nr]);
+ return sprintf(buf, "%d\n", val);
}

static ssize_t temp_min_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int nr = to_sensor_dev_attr(attr)->index;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_min[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->temp_min))
+ val = TEMP_FROM_REG(data->temp_min[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->temp_min))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -976,21 +1031,28 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_max[nr]));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->temp_max))
+ val = TEMP_FROM_REG(data->temp_max[nr]);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->temp_max))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1021,21 +1083,28 @@ static ssize_t pwm_auto_channels_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- int nr = to_sensor_dev_attr(attr)->index;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", ZONE_FROM_REG(data->autofan[nr].config));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->autofan))
+ val = ZONE_FROM_REG(data->autofan[nr].config);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->autofan))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1052,21 +1121,28 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", PWM_FROM_REG(data->autofan[nr].min_pwm));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->autofan))
+ val = PWM_FROM_REG(data->autofan[nr].min_pwm);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
unsigned long val;
int err;

+ if (nr >= ARRAY_SIZE(data->autofan))
+ return -EINVAL;
+
err = kstrtoul(buf, 10, &val);
if (err)
return err;
@@ -1083,22 +1159,29 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", data->autofan[nr].min_off);
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->autofan))
+ val = data->autofan[nr].min_off;
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
u8 tmp;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->autofan))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1130,23 +1213,30 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit) -
- HYST_FROM_REG(data->zone[nr].hyst));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->zone))
+ val = TEMP_FROM_REG(data->zone[nr].limit) -
+ HYST_FROM_REG(data->zone[nr].hyst);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
int min;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->zone))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1170,21 +1260,28 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->zone))
+ val = TEMP_FROM_REG(data->zone[nr].limit);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->zone))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1194,7 +1291,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,23 +1307,30 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].limit) +
- RANGE_FROM_REG(data->zone[nr].range));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->zone))
+ val = TEMP_FROM_REG(data->zone[nr].limit) +
+ RANGE_FROM_REG(data->zone[nr].range);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
int min;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->zone))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return err;
@@ -1247,21 +1351,28 @@ 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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = lm85_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->zone[nr].critical));
+ int val = 0;
+
+ if (nr < ARRAY_SIZE(data->zone))
+ val = TEMP_FROM_REG(data->zone[nr].critical);
+ return sprintf(buf, "%d\n", val);
}

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;
+ unsigned int nr = to_sensor_dev_attr(attr)->index;
struct lm85_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
long val;
int err;

+ if (nr >= ARRAY_SIZE(data->zone))
+ return -EINVAL;
+
err = kstrtol(buf, 10, &val);
if (err)
return 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

Next message: Paul Moore: "Re: [PATCH v1 0/2] two suggested iouring op audit updates"
Previous message: Jeff LaBundy: "Re: [PATCH] dt-bindings: input: azoteq: Fix differing types"
Next in thread: Guenter Roeck: "Re: [PATCH] lm85: Bounds check to_sensor_dev_attr()->index usage"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]