hwmon: (pmbus) Fix type confusion in notification logic

Sashiko reports:

At the start of the loop in pmbus_notify(), the code unconditionally casts
every attribute to a struct sensor_device_attribute:

drivers/hwmon/pmbus/pmbus_core.c:pmbus_notify() {
    for (i = 0; i < data->num_attributes; i++) {
        struct device_attribute *da = to_dev_attr(data->group.attrs[i]);
        struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
        int index = attr->index;
...
}

However, data->group.attrs can contain other types like struct
pmbus_samples_reg or struct pmbus_sensor, which only embed a base
struct device_attribute.

If da is a struct pmbus_samples_reg, dev_attr is the last member. Casting
it to struct sensor_device_attribute and reading the index field appears
to access memory past the end of the allocation, which might trigger a
slab-out-of-bounds read.

Additionally, if da is a struct pmbus_sensor, casting it causes the index
field to overlap with the page, phase, and reg fields. Could this produce
a garbage mask on little-endian systems that spuriously matches the target
reg, page, and flags during an alert?

Fix the problem by using struct sensor_device_attr in struct pmbus_sensor
and struct pmbus_label. Since those attributes never trigger a
notification, set the value of attr->index to -1 for them. Use this value
to distinguish from boolean attributes which _can_ trigger a notification
and use the index field to encode mask, page, and register values.

Fixes: f469bde9afd1 ("hwmon: (pmbus/core) Notify hwmon events")
Cc: Vincent Jardin <vjardin@free.fr>
Tested-by: Vincent Jardin <vjardin@free.fr>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 095e128709af44624552c843b28b068aae2650c4..3b219ed5386fba399b10696062517297eb5c65c6 100644 (file)
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -45,7 +45,7 @@ module_param(wp, int, 0444);
 struct pmbus_sensor {
        struct pmbus_sensor *next;
        char name[PMBUS_NAME_SIZE];     /* sysfs sensor name */
-       struct device_attribute attribute;
+       struct sensor_device_attribute attribute;
        u8 page;                /* page number */
        u8 phase;               /* phase number, 0xff for all phases */
        u16 reg;                /* register */
@@ -68,7 +68,7 @@ struct pmbus_boolean {
 
 struct pmbus_label {
        char name[PMBUS_NAME_SIZE];     /* sysfs label name */
-       struct device_attribute attribute;
+       struct sensor_device_attribute attribute;
        char label[PMBUS_NAME_SIZE];    /* label */
 };
 #define to_pmbus_label(_attr) \
@@ -1241,7 +1241,8 @@ static ssize_t pmbus_show_sensor(struct device *dev,
                                 struct device_attribute *devattr, char *buf)
 {
        struct i2c_client *client = to_i2c_client(dev->parent);
-       struct pmbus_sensor *sensor = to_pmbus_sensor(devattr);
+       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+       struct pmbus_sensor *sensor = to_pmbus_sensor(attr);
        struct pmbus_data *data = i2c_get_clientdata(client);
        s64 val;
 
@@ -1261,7 +1262,8 @@ static ssize_t pmbus_set_sensor(struct device *dev,
 {
        struct i2c_client *client = to_i2c_client(dev->parent);
        struct pmbus_data *data = i2c_get_clientdata(client);
-       struct pmbus_sensor *sensor = to_pmbus_sensor(devattr);
+       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+       struct pmbus_sensor *sensor = to_pmbus_sensor(attr);
        s64 val;
        int ret;
        u16 regval;
@@ -1283,7 +1285,8 @@ static ssize_t pmbus_set_sensor(struct device *dev,
 static ssize_t pmbus_show_label(struct device *dev,
                                struct device_attribute *da, char *buf)
 {
-       struct pmbus_label *label = to_pmbus_label(da);
+       struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+       struct pmbus_label *label = to_pmbus_label(attr);
 
        return sysfs_emit(buf, "%s\n", label->label);
 }
@@ -1436,8 +1439,8 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
                                             bool update, bool readonly,
                                             bool writeonly, bool convert)
 {
+       struct sensor_device_attribute *a;
        struct pmbus_sensor *sensor;
-       struct device_attribute *a;
 
        sensor = devm_kzalloc(data->dev, sizeof(*sensor), GFP_KERNEL);
        if (!sensor)
@@ -1461,12 +1464,11 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
        sensor->update = update;
        sensor->convert = convert;
        sensor->data = -ENODATA;
-       pmbus_dev_attr_init(a, sensor->name,
-                           readonly ? 0444 : 0644,
-                           writeonly ? pmbus_show_zero : pmbus_show_sensor,
-                           pmbus_set_sensor);
+       pmbus_attr_init(a, sensor->name, readonly ? 0444 : 0644,
+                       writeonly ? pmbus_show_zero : pmbus_show_sensor,
+                       pmbus_set_sensor, -1);
 
-       if (pmbus_add_attribute(data, &a->attr))
+       if (pmbus_add_attribute(data, &a->dev_attr.attr))
                return NULL;
 
        sensor->next = data->sensors;
@@ -1483,8 +1485,8 @@ static int pmbus_add_label(struct pmbus_data *data,
                           const char *name, int seq,
                           const char *lstring, int index, int phase)
 {
+       struct sensor_device_attribute *a;
        struct pmbus_label *label;
-       struct device_attribute *a;
 
        label = devm_kzalloc(data->dev, sizeof(*label), GFP_KERNEL);
        if (!label)
@@ -1508,8 +1510,8 @@ static int pmbus_add_label(struct pmbus_data *data,
                                 lstring, index, phase);
        }
 
-       pmbus_dev_attr_init(a, label->name, 0444, pmbus_show_label, NULL);
-       return pmbus_add_attribute(data, &a->attr);
+       pmbus_attr_init(a, label->name, 0444, pmbus_show_label, NULL, -1);
+       return pmbus_add_attribute(data, &a->dev_attr.attr);
 }
 
 /*
@@ -2397,7 +2399,7 @@ struct pmbus_samples_attr {
 struct pmbus_samples_reg {
        int page;
        struct pmbus_samples_attr *attr;
-       struct device_attribute dev_attr;
+       struct sensor_device_attribute attribute;
 };
 
 static struct pmbus_samples_attr pmbus_samples_registers[] = {
@@ -2419,14 +2421,15 @@ static struct pmbus_samples_attr pmbus_samples_registers[] = {
        }
 };
 
-#define to_samples_reg(x) container_of(x, struct pmbus_samples_reg, dev_attr)
+#define to_samples_reg(x) container_of(x, struct pmbus_samples_reg, attribute)
 
 static ssize_t pmbus_show_samples(struct device *dev,
                                  struct device_attribute *devattr, char *buf)
 {
        int val;
        struct i2c_client *client = to_i2c_client(dev->parent);
-       struct pmbus_samples_reg *reg = to_samples_reg(devattr);
+       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+       struct pmbus_samples_reg *reg = to_samples_reg(attr);
 
        scoped_guard(pmbus_lock, client) {
                val = _pmbus_read_word_data(client, reg->page, 0xff, reg->attr->reg);
@@ -2444,7 +2447,8 @@ static ssize_t pmbus_set_samples(struct device *dev,
        int ret;
        long val;
        struct i2c_client *client = to_i2c_client(dev->parent);
-       struct pmbus_samples_reg *reg = to_samples_reg(devattr);
+       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+       struct pmbus_samples_reg *reg = to_samples_reg(attr);
 
        if (kstrtol(buf, 0, &val) < 0)
                return -EINVAL;
@@ -2459,6 +2463,7 @@ static ssize_t pmbus_set_samples(struct device *dev,
 static int pmbus_add_samples_attr(struct pmbus_data *data, int page,
                                  struct pmbus_samples_attr *attr)
 {
+       struct sensor_device_attribute *a;
        struct pmbus_samples_reg *reg;
 
        reg = devm_kzalloc(data->dev, sizeof(*reg), GFP_KERNEL);
@@ -2468,10 +2473,12 @@ static int pmbus_add_samples_attr(struct pmbus_data *data, int page,
        reg->attr = attr;
        reg->page = page;
 
-       pmbus_dev_attr_init(&reg->dev_attr, attr->name, 0644,
-                           pmbus_show_samples, pmbus_set_samples);
+       a = &reg->attribute;
+
+       pmbus_attr_init(a, attr->name, 0644,
+                       pmbus_show_samples, pmbus_set_samples, -1);
 
-       return pmbus_add_attribute(data, &reg->dev_attr.attr);
+       return pmbus_add_attribute(data, &a->dev_attr.attr);
 }
 
 static int pmbus_add_samples_attributes(struct i2c_client *client,
@@ -2979,9 +2986,15 @@ static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
                struct device_attribute *da = to_dev_attr(data->group.attrs[i]);
                struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
                int index = attr->index;
-               u16 smask = pb_index_to_mask(index);
-               u8 spage = pb_index_to_page(index);
-               u16 sreg = pb_index_to_reg(index);
+               u16 smask, sreg;
+               u8 spage;
+
+               if (index == -1)
+                       continue;
+
+               smask = pb_index_to_mask(index);
+               spage = pb_index_to_page(index);
+               sreg = pb_index_to_reg(index);
 
                if (reg == sreg && page == spage && (smask & flags)) {
                        dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);