[lm-sensors] [PATCH] hwmon: fix common race conditions, batch 2

Mark M. Hoffman mhoffman at lightlink.com
Sun Apr 6 20:07:21 CEST 2008


Hi:

 atxp1.c    |    2 ++
 coretemp.c |    2 ++
 dme1737.c  |    6 ++++++
 f71882fg.c |   14 ++++++++++----
 gl518sm.c  |   18 ++++++++++++++----
 gl520sm.c  |   16 ++++++++++++----
 it87.c     |   26 ++++++++++++++++----------
 7 files changed, 62 insertions(+), 22 deletions(-)

NB: reviewers are also encouraged to have a look at these files, in which I
found no race condtions of this class:

ds1621.c
f71805f.c
f75375s.c
fscher.c
fschmd.c
fscpos.c
i5k_amb.c
ibmpex.c

commit e62da7be499f5553ce682aa6830a4e51ab293619
Author: Mark M. Hoffman <mhoffman at lightlink.com>
Date:   Sun Apr 6 13:45:00 2008 -0400

    hwmon: fix common race conditions, batch 2
    
    Most hwmon drivers have one or more race conditions that could cause the driver
    to display incorrect data; in the absolute worst case, these races could allow
    divide-by-zero/OOPS.  The most common of these involves a race between setting
    a fan divider and displaying the fan data (RPMs).  This patch fixes these race
    conditions by taking the update lock (or equivalent) as necessary.
    
    Signed-off-by: Mark M. Hoffman <mhoffman at lightlink.com>

diff --git a/drivers/hwmon/atxp1.c b/drivers/hwmon/atxp1.c
index 01c17e3..aca58c9 100644
--- a/drivers/hwmon/atxp1.c
+++ b/drivers/hwmon/atxp1.c
@@ -136,10 +136,12 @@ static ssize_t atxp1_storevcore(struct device *dev, struct device_attribute *att
 	}
 
 	/* If output enabled, use control register value. Otherwise original CPU VID */
+	mutex_lock(&data->update_lock);
 	if (data->reg.vid & ATXP1_VIDENA)
 		cvid = data->reg.vid & ATXP1_VIDMASK;
 	else
 		cvid = data->reg.cpu_vid;
+	mutex_unlock(&data->update_lock);
 
 	/* Nothing changed, aborting */
 	if (vid == cvid)
diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 70239ac..1fbaeff 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -93,12 +93,14 @@ static ssize_t show_temp(struct device *dev,
 	struct coretemp_data *data = coretemp_update_device(dev);
 	int err;
 
+	mutex_lock(&data->update_lock);
 	if (attr->index == SHOW_TEMP)
 		err = data->valid ? sprintf(buf, "%d\n", data->temp) : -EAGAIN;
 	else if (attr->index == SHOW_TJMAX)
 		err = sprintf(buf, "%d\n", data->tjmax);
 	else
 		err = sprintf(buf, "%d\n", data->ttarget);
+	mutex_unlock(&data->update_lock);
 	return err;
 }
 
diff --git a/drivers/hwmon/dme1737.c b/drivers/hwmon/dme1737.c
index 7673f65..28c9d25 100644
--- a/drivers/hwmon/dme1737.c
+++ b/drivers/hwmon/dme1737.c
@@ -878,6 +878,7 @@ static ssize_t show_zone(struct device *dev, struct device_attribute *attr,
 	int fn = sensor_attr_2->nr;
 	int res;
 
+	mutex_lock(&data->update_lock);
 	switch (fn) {
 	case SYS_ZONE_AUTO_CHANNELS_TEMP:
 		/* check config2 for non-standard temp-to-zone mapping */
@@ -906,6 +907,7 @@ static ssize_t show_zone(struct device *dev, struct device_attribute *attr,
 		res = 0;
 		dev_dbg(dev, "Unknown function %d.\n", fn);
 	}
+	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", res);
 }
@@ -987,6 +989,7 @@ static ssize_t show_fan(struct device *dev, struct device_attribute *attr,
 	int fn = sensor_attr_2->nr;
 	int res;
 
+	mutex_lock(&data->update_lock);
 	switch (fn) {
 	case SYS_FAN_INPUT:
 		res = FAN_FROM_REG(data->fan[ix],
@@ -1013,6 +1016,7 @@ static ssize_t show_fan(struct device *dev, struct device_attribute *attr,
 		res = 0;
 		dev_dbg(dev, "Unknown function %d.\n", fn);
 	}
+	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", res);
 }
@@ -1099,6 +1103,7 @@ static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
 	int fn = sensor_attr_2->nr;
 	int res;
 
+	mutex_lock(&data->update_lock);
 	switch (fn) {
 	case SYS_PWM:
 		if (PWM_EN_FROM_REG(data->pwm_config[ix]) == 0) {
@@ -1149,6 +1154,7 @@ static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
 		res = 0;
 		dev_dbg(dev, "Unknown function %d.\n", fn);
 	}
+	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", res);
 }
diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
index cbeb498..a97ebde 100644
--- a/drivers/hwmon/f71882fg.c
+++ b/drivers/hwmon/f71882fg.c
@@ -593,9 +593,12 @@ static ssize_t show_temp_max_hyst(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = f71882fg_update_device(dev);
 	int nr = to_sensor_dev_attr(devattr)->index;
+	int res;
 
-	return sprintf(buf, "%d\n",
-		(data->temp_high[nr] - data->temp_hyst[nr]) * 1000);
+	mutex_lock(&data->update_lock);
+	res = (data->temp_high[nr] - data->temp_hyst[nr]) * 1000;
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", res);
 }
 
 static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
@@ -670,9 +673,12 @@ static ssize_t show_temp_crit_hyst(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = f71882fg_update_device(dev);
 	int nr = to_sensor_dev_attr(devattr)->index;
+	int res;
 
-	return sprintf(buf, "%d\n",
-		(data->temp_ovt[nr] - data->temp_hyst[nr]) * 1000);
+	mutex_lock(&data->update_lock);
+	res = (data->temp_ovt[nr] - data->temp_hyst[nr]) * 1000;
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", res);
 }
 
 static ssize_t show_temp_type(struct device *dev, struct device_attribute
diff --git a/drivers/hwmon/gl518sm.c b/drivers/hwmon/gl518sm.c
index 33e9e8a..8b83690 100644
--- a/drivers/hwmon/gl518sm.c
+++ b/drivers/hwmon/gl518sm.c
@@ -191,8 +191,13 @@ static ssize_t show_fan_input(struct device *dev,
 {
 	int nr = to_sensor_dev_attr(attr)->index;
 	struct gl518_data *data = gl518_update_device(dev);
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_in[nr],
-					DIV_FROM_REG(data->fan_div[nr])));
+	int result;
+
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan_in[nr],
+				DIV_FROM_REG(data->fan_div[nr]));
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 
 static ssize_t show_fan_min(struct device *dev,
@@ -200,8 +205,13 @@ static ssize_t show_fan_min(struct device *dev,
 {
 	int nr = to_sensor_dev_attr(attr)->index;
 	struct gl518_data *data = gl518_update_device(dev);
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[nr],
-					DIV_FROM_REG(data->fan_div[nr])));
+	int result;
+
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan_min[nr],
+				DIV_FROM_REG(data->fan_div[nr]));
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 
 static ssize_t show_fan_div(struct device *dev,
diff --git a/drivers/hwmon/gl520sm.c b/drivers/hwmon/gl520sm.c
index 8984ef1..804018d 100644
--- a/drivers/hwmon/gl520sm.c
+++ b/drivers/hwmon/gl520sm.c
@@ -273,9 +273,13 @@ static ssize_t get_fan_input(struct device *dev, struct device_attribute *attr,
 {
 	int n = to_sensor_dev_attr(attr)->index;
 	struct gl520_data *data = gl520_update_device(dev);
+	int result;
 
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_input[n],
-						 data->fan_div[n]));
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan_input[n],
+				data->fan_div[n]);
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 
 static ssize_t get_fan_min(struct device *dev, struct device_attribute *attr,
@@ -283,9 +287,13 @@ static ssize_t get_fan_min(struct device *dev, struct device_attribute *attr,
 {
 	int n = to_sensor_dev_attr(attr)->index;
 	struct gl520_data *data = gl520_update_device(dev);
+	int result;
 
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[n],
-						 data->fan_div[n]));
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan_min[n],
+				 data->fan_div[n]);
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 
 static ssize_t get_fan_div(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index e12c132..45a216f 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -502,22 +502,28 @@ show_sensor_offset(3);
 static ssize_t show_fan(struct device *dev, struct device_attribute *attr,
 		char *buf)
 {
-	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
-	int nr = sensor_attr->index;
-
+	int nr = to_sensor_dev_attr(attr)->index;
 	struct it87_data *data = it87_update_device(dev);
-	return sprintf(buf,"%d\n", FAN_FROM_REG(data->fan[nr], 
-				DIV_FROM_REG(data->fan_div[nr])));
+	int result;
+
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan[nr],
+			DIV_FROM_REG(data->fan_div[nr]));
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 static ssize_t show_fan_min(struct device *dev, struct device_attribute *attr,
 		char *buf)
 {
-	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
-	int nr = sensor_attr->index;
-
+	int nr = to_sensor_dev_attr(attr)->index;
 	struct it87_data *data = it87_update_device(dev);
-	return sprintf(buf,"%d\n",
-		FAN_FROM_REG(data->fan_min[nr], DIV_FROM_REG(data->fan_div[nr])));
+	int result;
+
+	mutex_lock(&data->update_lock);
+	result = FAN_FROM_REG(data->fan_min[nr],
+			DIV_FROM_REG(data->fan_div[nr]));
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", result);
 }
 static ssize_t show_fan_div(struct device *dev, struct device_attribute *attr,
 		char *buf)

Regards,

-- 
Mark M. Hoffman
mhoffman at lightlink.com





More information about the lm-sensors mailing list