Am 03.11.2015 23:14, schrieb Dan Carpenter:
> This is to address a static checker warning about an underflow in
> imx_set_trip_temp(). The checker is complaining that we have a user
> supplied value for "temp" from kstrtoul() where we treat it as signed,
> we cap the upper but we accept negative values.
>
> This looks unintentional since the caller is using unsigned longs to
> represent the temperature. Let's change it to int and reject negatives
> in the caller.
>
> Also I changed it to reject negative "trip" values as well.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> Someday we will use super cooled CPUs and we will need to rethink this
> code. :)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index d9e525c..151a630 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -664,7 +664,7 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
> {
> struct thermal_zone_device *tz = to_thermal_zone(dev);
> int trip, ret;
> - unsigned long temperature;
> + int temperature;
>
> if (!tz->ops->set_trip_temp)
> return -EPERM;
> @@ -672,7 +672,9 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
> if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
> return -EINVAL;
>
> - if (kstrtoul(buf, 10, &temperature))
> + if (kstrtoint(buf, 10, &temperature))
> + return -EINVAL;
> + if (trip < 0 || temperature < 0)
> return -EINVAL;
>
> ret = tz->ops->set_trip_temp(tz, trip, temperature);
IMHO the test should be near the point where the value is generated.
if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
return -EINVAL;
if (trip < 0)
return -EINVAL;
if (kstrtoint(buf, 10, &temperature))
return -EINVAL;
if (temperature < 0)
return -EINVAL;
That way it is easily visible under what condition -EINVAL is generated (to many).
hope that helps,
re,
wh
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
