linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] hwmon: (dme1737) Fix overflows seen when writing into limit attributes
@ 2016-12-27 22:15 Guenter Roeck
  2016-12-27 22:15 ` [PATCH v2 2/3] hwmon: (gl518sm) " Guenter Roeck
  2016-12-27 22:15 ` [PATCH v2 3/3] hwmon: (gl520sm) Fix overflows and crash " Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Guenter Roeck @ 2016-12-27 22:15 UTC (permalink / raw)
  To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck, Juerg Haefliger

Writes into voltage limit, temperature limit, temperature hysteresis,
and temperature zone attributes can overflow due to unclamped parameters
to multiplications, additions, and subtractions.

Cc: Juerg Haefliger <juergh@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Reorganize code to avoid double clamp in set_zone().

 drivers/hwmon/dme1737.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/dme1737.c b/drivers/hwmon/dme1737.c
index 8763c4a8280c..dc7c26c68e6d 100644
--- a/drivers/hwmon/dme1737.c
+++ b/drivers/hwmon/dme1737.c
@@ -279,7 +279,8 @@ static inline int IN_FROM_REG(int reg, int nominal, int res)
 
 static inline int IN_TO_REG(long val, int nominal)
 {
-	return clamp_val((val * 192 + nominal / 2) / nominal, 0, 255);
+	val = clamp_val(val, 0, 255 * nominal / 192);
+	return DIV_ROUND_CLOSEST(val * 192, nominal);
 }
 
 /*
@@ -295,7 +296,8 @@ static inline int TEMP_FROM_REG(int reg, int res)
 
 static inline int TEMP_TO_REG(long val)
 {
-	return clamp_val((val < 0 ? val - 500 : val + 500) / 1000, -128, 127);
+	val = clamp_val(val, -128000, 127000);
+	return DIV_ROUND_CLOSEST(val, 1000);
 }
 
 /* Temperature range */
@@ -331,9 +333,10 @@ static inline int TEMP_HYST_FROM_REG(int reg, int ix)
 	return (((ix == 1) ? reg : reg >> 4) & 0x0f) * 1000;
 }
 
-static inline int TEMP_HYST_TO_REG(long val, int ix, int reg)
+static inline int TEMP_HYST_TO_REG(int temp, long hyst, int ix, int reg)
 {
-	int hyst = clamp_val((val + 500) / 1000, 0, 15);
+	hyst = clamp_val(hyst, temp - 15000, temp);
+	hyst = DIV_ROUND_CLOSEST(temp - hyst, 1000);
 
 	return (ix == 1) ? (reg & 0xf0) | hyst : (reg & 0x0f) | (hyst << 4);
 }
@@ -1022,7 +1025,9 @@ static ssize_t set_zone(struct device *dev, struct device_attribute *attr,
 	int ix = sensor_attr_2->index;
 	int fn = sensor_attr_2->nr;
 	long val;
+	int temp;
 	int err;
+	u8 reg;
 
 	err = kstrtol(buf, 10, &val);
 	if (err)
@@ -1035,10 +1040,9 @@ static ssize_t set_zone(struct device *dev, struct device_attribute *attr,
 		data->zone_low[ix] = dme1737_read(data,
 						  DME1737_REG_ZONE_LOW(ix));
 		/* Modify the temp hyst value */
-		data->zone_hyst[ix == 2] = TEMP_HYST_TO_REG(
-					TEMP_FROM_REG(data->zone_low[ix], 8) -
-					val, ix, dme1737_read(data,
-					DME1737_REG_ZONE_HYST(ix == 2)));
+		temp = TEMP_FROM_REG(data->zone_low[ix], 8);
+		reg = dme1737_read(data, DME1737_REG_ZONE_HYST(ix == 2));
+		data->zone_hyst[ix == 2] = TEMP_HYST_TO_REG(temp, val, ix, reg);
 		dme1737_write(data, DME1737_REG_ZONE_HYST(ix == 2),
 			      data->zone_hyst[ix == 2]);
 		break;
@@ -1055,10 +1059,10 @@ static ssize_t set_zone(struct device *dev, struct device_attribute *attr,
 		 * Modify the temp range value (which is stored in the upper
 		 * nibble of the pwm_freq register)
 		 */
-		data->pwm_freq[ix] = TEMP_RANGE_TO_REG(val -
-					TEMP_FROM_REG(data->zone_low[ix], 8),
-					dme1737_read(data,
-					DME1737_REG_PWM_FREQ(ix)));
+		temp = TEMP_FROM_REG(data->zone_low[ix], 8);
+		val = clamp_val(val, temp, temp + 80000);
+		reg = dme1737_read(data, DME1737_REG_PWM_FREQ(ix));
+		data->pwm_freq[ix] = TEMP_RANGE_TO_REG(val - temp, reg);
 		dme1737_write(data, DME1737_REG_PWM_FREQ(ix),
 			      data->pwm_freq[ix]);
 		break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH v2 2/3] hwmon: (gl518sm) Fix overflows seen when writing into limit attributes
  2016-12-27 22:15 [PATCH v2 1/3] hwmon: (dme1737) Fix overflows seen when writing into limit attributes Guenter Roeck
@ 2016-12-27 22:15 ` Guenter Roeck
  2016-12-27 22:15 ` [PATCH v2 3/3] hwmon: (gl520sm) Fix overflows and crash " Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2016-12-27 22:15 UTC (permalink / raw)
  To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck

Writes into limit attributes can overflow due to additions and
multiplications with unchecked parameters.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Improve alignment
    Use DIV_ROUND_CLOSEST() more consistently

 drivers/hwmon/gl518sm.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/gl518sm.c b/drivers/hwmon/gl518sm.c
index 0212c8317bca..b267510daeb2 100644
--- a/drivers/hwmon/gl518sm.c
+++ b/drivers/hwmon/gl518sm.c
@@ -86,9 +86,8 @@ enum chips { gl518sm_r00, gl518sm_r80 };
 #define BOOL_FROM_REG(val)	((val) ? 0 : 1)
 #define BOOL_TO_REG(val)	((val) ? 0 : 1)
 
-#define TEMP_TO_REG(val)	clamp_val(((((val) < 0 ? \
-				(val) - 500 : \
-				(val) + 500) / 1000) + 119), 0, 255)
+#define TEMP_CLAMP(val)		clamp_val(val, -119000, 136000)
+#define TEMP_TO_REG(val)	(DIV_ROUND_CLOSEST(TEMP_CLAMP(val), 1000) + 119)
 #define TEMP_FROM_REG(val)	(((val) - 119) * 1000)
 
 static inline u8 FAN_TO_REG(long rpm, int div)
@@ -101,11 +100,13 @@ static inline u8 FAN_TO_REG(long rpm, int div)
 }
 #define FAN_FROM_REG(val, div)	((val) == 0 ? 0 : (480000 / ((val) * (div))))
 
-#define IN_TO_REG(val)		clamp_val((((val) + 9) / 19), 0, 255)
+#define IN_CLAMP(val)		clamp_val(val, 0, 255 * 19)
+#define IN_TO_REG(val)		DIV_ROUND_CLOSEST(IN_CLAMP(val), 19)
 #define IN_FROM_REG(val)	((val) * 19)
 
-#define VDD_TO_REG(val)		clamp_val((((val) * 4 + 47) / 95), 0, 255)
-#define VDD_FROM_REG(val)	(((val) * 95 + 2) / 4)
+#define VDD_CLAMP(val)		clamp_val(val, 0, 255 * 95 / 4)
+#define VDD_TO_REG(val)		DIV_ROUND_CLOSEST(VDD_CLAMP(val) * 4, 95)
+#define VDD_FROM_REG(val)	DIV_ROUND_CLOSEST((val) * 95, 4)
 
 #define DIV_FROM_REG(val)	(1 << (val))
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH v2 3/3] hwmon: (gl520sm) Fix overflows and crash seen when writing into limit attributes
  2016-12-27 22:15 [PATCH v2 1/3] hwmon: (dme1737) Fix overflows seen when writing into limit attributes Guenter Roeck
  2016-12-27 22:15 ` [PATCH v2 2/3] hwmon: (gl518sm) " Guenter Roeck
@ 2016-12-27 22:15 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2016-12-27 22:15 UTC (permalink / raw)
  To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck

Writes into limit attributes can overflow due to multplications and
additions with unbound input values. Writing into fan limit attributes
can result in a crash with a division by zero if very large values are
written and the fan divider is larger than 1.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Catch overflow (and resulting crash) seen when writing certain values
    into fan limit attributes

 drivers/hwmon/gl520sm.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/gl520sm.c b/drivers/hwmon/gl520sm.c
index dee93ec87d02..84e0994aafdd 100644
--- a/drivers/hwmon/gl520sm.c
+++ b/drivers/hwmon/gl520sm.c
@@ -208,11 +208,13 @@ static ssize_t get_cpu_vid(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR(cpu0_vid, S_IRUGO, get_cpu_vid, NULL);
 
-#define VDD_FROM_REG(val) (((val) * 95 + 2) / 4)
-#define VDD_TO_REG(val) clamp_val((((val) * 4 + 47) / 95), 0, 255)
+#define VDD_FROM_REG(val)	DIV_ROUND_CLOSEST((val) * 95, 4)
+#define VDD_CLAMP(val)		clamp_val(val, 0, 255 * 95 / 4)
+#define VDD_TO_REG(val)		DIV_ROUND_CLOSEST(VDD_CLAMP(val) * 4, 95)
 
-#define IN_FROM_REG(val) ((val) * 19)
-#define IN_TO_REG(val) clamp_val((((val) + 9) / 19), 0, 255)
+#define IN_FROM_REG(val)	((val) * 19)
+#define IN_CLAMP(val)		clamp_val(val, 0, 255 * 19)
+#define IN_TO_REG(val)		DIV_ROUND_CLOSEST(IN_CLAMP(val), 19)
 
 static ssize_t get_in_input(struct device *dev, struct device_attribute *attr,
 			    char *buf)
@@ -349,8 +351,13 @@ static SENSOR_DEVICE_ATTR(in4_max, S_IRUGO | S_IWUSR,
 
 #define DIV_FROM_REG(val) (1 << (val))
 #define FAN_FROM_REG(val, div) ((val) == 0 ? 0 : (480000 / ((val) << (div))))
-#define FAN_TO_REG(val, div) ((val) <= 0 ? 0 : \
-	clamp_val((480000 + ((val) << ((div)-1))) / ((val) << (div)), 1, 255))
+
+#define FAN_BASE(div)		(480000 >> (div))
+#define FAN_CLAMP(val, div)	clamp_val(val, FAN_BASE(div) / 255, \
+					  FAN_BASE(div))
+#define FAN_TO_REG(val, div)	((val) == 0 ? 0 : \
+				 DIV_ROUND_CLOSEST(480000, \
+						FAN_CLAMP(val, div) << (div)))
 
 static ssize_t get_fan_input(struct device *dev, struct device_attribute *attr,
 			     char *buf)
@@ -513,9 +520,9 @@ static SENSOR_DEVICE_ATTR(fan2_div, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(fan1_off, S_IRUGO | S_IWUSR,
 		get_fan_off, set_fan_off);
 
-#define TEMP_FROM_REG(val) (((val) - 130) * 1000)
-#define TEMP_TO_REG(val) clamp_val(((((val) < 0 ? \
-			(val) - 500 : (val) + 500) / 1000) + 130), 0, 255)
+#define TEMP_FROM_REG(val)	(((val) - 130) * 1000)
+#define TEMP_CLAMP(val)		clamp_val(val, -130000, 125000)
+#define TEMP_TO_REG(val)	(DIV_ROUND_CLOSEST(TEMP_CLAMP(val), 1000) + 130)
 
 static ssize_t get_temp_input(struct device *dev, struct device_attribute *attr,
 			      char *buf)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-27 22:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-27 22:15 [PATCH v2 1/3] hwmon: (dme1737) Fix overflows seen when writing into limit attributes Guenter Roeck
2016-12-27 22:15 ` [PATCH v2 2/3] hwmon: (gl518sm) " Guenter Roeck
2016-12-27 22:15 ` [PATCH v2 3/3] hwmon: (gl520sm) Fix overflows and crash " Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).