Skip to contentSkip to conversation

hwmon: (ds620): Check data pointer before dereference it

PATCH In review Axel Lin 2 participants 4 messages Jul 12, 2014 Hardware monitoring

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 hwmon: (ds620): Check data pointer before dereference it

data can be an ERR_PTR, so the client assignment must only happen after the ERR_PTR check.

---
 drivers/hwmon/ds620.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/hwmon/ds620.c +9 -2
index 1bdb6246..65e01a8 100644
@@ -159,7 +159,12 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *da,
 
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	struct ds620_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
+	struct i2c_client *client;
+
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	client = data->client;
 
 	res = kstrtol(buf, 10, &val);
 
@@ -181,13 +186,15 @@ static ssize_t show_alarm(struct device *dev, struct device_attribute *da,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	struct ds620_data *data = ds620_update_client(dev);
-	struct i2c_client *client = data->client;
+	struct i2c_client *client;
 	u16 conf, new_conf;
 	int res;
 
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 
+	client = data->client;
+
 	/* reset alarms if necessary */
 	res = i2c_smbus_read_word_swapped(client, DS620_REG_CONF);
 	if (res < 0)
Signature · 9 lines
1.9.1




_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
maintainer
#1 lore raw
Quoted 14 lines · Axel Lin

data can be an ERR_PTR, so the client assignment must only happen after the ERR_PTR check.

---
  drivers/hwmon/ds620.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
drivers/hwmon/ds620.c +0 -0
index 1bdb6246..65e01a8 100644

drivers/hwmon/ds620.c static ssize_t set_temp(struct device *dev, struct device_attribute *da, · line 166

Quoted 9 lines
drivers/hwmon/ds620.c +5 -1
@@ -159,7 +159,12 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *da,
 
  	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
  	struct ds620_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
+	struct i2c_client *client;
+
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+

Hi Axel,

This check is not needed; data is from dev_get_drvdata() and that is not problematic.

Guenter

Quoted 12 lines
drivers/hwmon/ds620.c +2 -1
+	client = data->client;
 
  	res = kstrtol(buf, 10, &val);
 
@@ -181,13 +186,15 @@ static ssize_t show_alarm(struct device *dev, struct device_attribute *da,
  {
  	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
  	struct ds620_data *data = ds620_update_client(dev);
-	struct i2c_client *client = data->client;
+	struct i2c_client *client;
  	u16 conf, new_conf;

drivers/hwmon/ds620.c static ssize_t show_alarm(struct device *dev, struct device_attribute *da, · line 200

Quoted 10 lines
drivers/hwmon/ds620.c +2 -0
  	int res;
 
  	if (IS_ERR(data))
  		return PTR_ERR(data);
 
+	client = data->client;
+
  	/* reset alarms if necessary */
  	res = i2c_smbus_read_word_swapped(client, DS620_REG_CONF);
  	if (res < 0)

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

maintainer
#2 lore raw
In reply to Axel Lin
Quoted 26 lines · Guenter Roeck
Quoted 14 lines · Axel Lin

data can be an ERR_PTR, so the client assignment must only happen after the ERR_PTR check.

---
  drivers/hwmon/ds620.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
drivers/hwmon/ds620.c +0 -0
index 1bdb6246..65e01a8 100644

drivers/hwmon/ds620.c static ssize_t set_temp(struct device *dev, struct device_attribute *da, · line 166

Quoted 9 lines
drivers/hwmon/ds620.c +5 -1
@@ -159,7 +159,12 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *da,
 
      struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
      struct ds620_data *data = dev_get_drvdata(dev);
-    struct i2c_client *client = data->client;
+    struct i2c_client *client;
+
+    if (IS_ERR(data))
+        return PTR_ERR(data);
+

Hi Axel,

Quoted 2 lines

This check is not needed; data is from dev_get_drvdata() and that is not problematic.

Never mind, though. I fixed it up myself and merged it with the original devm conversion patch.

Guenter

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

author
#3 lore raw
In reply to Axel Lin

2014-07-12 23:00 GMT+08:00 Guenter Roeck <[email protected]>:

Quoted 32 lines · Guenter Roeck
Quoted 31 lines · Axel Lin
Quoted 16 lines

data can be an ERR_PTR, so the client assignment must only happen after the
ERR_PTR check.

---
  drivers/hwmon/ds620.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
drivers/hwmon/ds620.c +0 -0
index 1bdb6246..65e01a8 100644

drivers/hwmon/ds620.c static ssize_t set_temp(struct device *dev, struct · line 167

Quoted 10 lines
drivers/hwmon/ds620.c +5 -1
@@ -159,7 +159,12 @@ static ssize_t set_temp(struct device *dev, struct
 device_attribute *da,
 
      struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
      struct ds620_data *data = dev_get_drvdata(dev);
-    struct i2c_client *client = data->client;
+    struct i2c_client *client;
+
+    if (IS_ERR(data))
+        return PTR_ERR(data);
+

Hi Axel,

Quoted 6 lines
Quoted 3 lines

This check is not needed; data is from dev_get_drvdata() and that is not problematic.

Never mind, though. I fixed it up myself and merged it with the original devm conversion patch.

Thank you, Guenter.

Axel

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors