[prev in list] [next in list] [prev in thread] [next in thread]
List: lm-sensors
Subject: Re: [lm-sensors] [PATCH] hwmon: (jc42) Fix integer overflow when writing hysteresis value
From: Jean Delvare <jdelvare () suse ! de>
Date: 2015-01-23 9:18:50
Message-ID: 20150123101850.6f7922a7 () endymion ! delvare
[Download RAW message or body]
Hi Guenter,
On Mon, 19 Jan 2015 09:19:52 -0800, Guenter Roeck wrote:
> Subtracting an unsigned long from a signed value causes an overflow with large
> values. Use clamp_val() to reduce the number range prior to subtracting it
> from the temperature limit.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/jc42.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
> index f8e3bbb..a46cb65 100644
> --- a/drivers/hwmon/jc42.c
> +++ b/drivers/hwmon/jc42.c
> @@ -312,7 +312,9 @@ static ssize_t set_temp_crit_hyst(struct device *dev,
> if (kstrtoul(buf, 10, &val) < 0)
> return -EINVAL;
>
> + val = clamp_val(val, 0, JC42_TEMP_MAX);
> diff = jc42_temp_from_reg(data->temp[t_crit]) - val;
> +
> hyst = 0;
> if (diff > 0) {
> if (diff < 2250)
Correct, however I am wondering why we reject negative hysteresis
values in the first place? My chips have the hysteresis delta at -3 °C
by default, I can set temp1_crit to -1 °C and temp1_crit_hyst will read
back -4 °C as expected, however the driver doesn't let me change the
hysteresis after that.
I agree that JC42_CAP_RANGE (extended temperature range) was certainly
meant for temp1_min rather than temp1_max or temp1_crit, but I still
believe the driver should not limit the user that way. I'll send a
patch, let me know what you think.
Anyway the fix above is correct, so:
Reviewed-by: Jean Delvare <jdelvare@suse.de>
--
Jean Delvare
SUSE L3 Support
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic