Hi Jean,
On 12/09/2016 01:49 AM, Jean Delvare wrote:
Hi Guenter,
On Sun, 4 Dec 2016 20:55:32 -0800, Guenter Roeck wrote:
Fix overflows seen when writing voltage and temperature limit attributes.
The value passed to DIV_ROUND_CLOSEST() needs to be clamped.
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
I think you can also add:
Fixes: 3434f3783580 ("hwmon: Driver for Nuvoton NCT7802Y")
---
drivers/hwmon/nct7802.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 3ce33d244cc0..6fe71cfe0320 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -326,8 +326,9 @@ static int nct7802_write_voltage(struct nct7802_data *data, int nr, int index,
int shift = 8 - REG_VOLTAGE_LIMIT_MSB_SHIFT[index - 1][nr];
int err;
- voltage = DIV_ROUND_CLOSEST(voltage, nct7802_vmul[nr]);
- voltage = clamp_val(voltage, 0, 0x3ff);
+ voltage = DIV_ROUND_CLOSEST(clamp_val(voltage, 0,
+ 0x3ff * nct7802_vmul[nr]),
+ nct7802_vmul[nr]);
As for previous patches, I think separate lines make the code more
readable.
Ok
mutex_lock(&data->access_lock);
err = regmap_write(data->regmap,
@@ -402,7 +403,7 @@ static ssize_t store_temp(struct device *dev, struct device_attribute *attr,
if (err < 0)
return err;
- val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -128, 127);
+ val = DIV_ROUND_CLOSEST(clamp_val(val, -127000, 127000), 1000);
You change the low limit from -128 to -127 without justification.
There is none. This is a bug.
err = regmap_write(data->regmap, nr, val & 0xff);
return err ? : count;
Looking at function nct7802_write_fan_min() I think an overflow can
happen here too, as DIV_ROUND_CLOSEST() is called before clamp_val().
Any reason why you did not fix that one?
Not really. The call is
DIV_ROUND_CLOSEST(1350000U, limit);
and thus won't overflow.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
