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 <li...@roeck-us.net>
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.
>
> 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.
>
> 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?
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
