thr3ads.net - Nouveau - [Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs [Jul 2017]

If this information is useful, please help other people find it:
Share via:

Dan Carpenter

2017-Jul-17 08:17 UTC

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

kstrtol() and friends can return -EINVAL or -ERANGE.  We have to test
for both, otherwise the value is possibly uninitialized.  Also in some
of these files we accidentally return "count" on error instead of a
negative error code.

Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index 7c965648df80..5e75af91c446 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -68,9 +68,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device *d,
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
 	long value;
+	int ret;
 
-	if (kstrtol(buf, 10, &value) == -EINVAL)
-		return count;
+	ret = kstrtol(buf, 10, &value);
+	if (ret)
+		return ret;
 
 	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST,
 			value / 1000);
@@ -101,9 +103,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct device
*d,
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
 	long value;
+	int ret;
 
-	if (kstrtol(buf, 10, &value) == -EINVAL)
-		return count;
+	ret = kstrtol(buf, 10, &value);
+	if (ret)
+		return ret;
 
 	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST_HYST,
 			value / 1000);
@@ -156,8 +160,9 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct
device_attribute *a,
 	long value;
 	int ret;
 
-	if (kstrtol(buf, 10, &value) == -EINVAL)
-		return -EINVAL;
+	ret = kstrtol(buf, 10, &value);
+	if (ret)
+		return ret;
 
 	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MIN_DUTY, value);
 	if (ret < 0)
@@ -179,8 +184,9 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct
device_attribute *a,
 	long value;
 	int ret;
 
-	if (kstrtol(buf, 10, &value) == -EINVAL)
-		return -EINVAL;
+	ret = kstrtol(buf, 10, &value);
+	if (ret)
+		return ret;
 
 	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MAX_DUTY, value);
 	if (ret < 0)

Pierre Moreau

2017-Jul-22 12:13 UTC

head link

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

Reviewed-by: Pierre Moreau <pierre.morrow at free.fr>

On 2017-07-17 — 11:17, Dan Carpenter wrote:> kstrtol() and friends can return -EINVAL or -ERANGE.  We have to test
> for both, otherwise the value is possibly uninitialized.  Also in some
> of these files we accidentally return "count" on error instead of
a
> negative error code.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> index 7c965648df80..5e75af91c446 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> @@ -68,9 +68,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device
*d,
>  	struct nouveau_drm *drm = nouveau_drm(dev);
>  	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  	long value;
> +	int ret;
>  
> -	if (kstrtol(buf, 10, &value) == -EINVAL)
> -		return count;
> +	ret = kstrtol(buf, 10, &value);
> +	if (ret)
> +		return ret;
>  
>  	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST,
>  			value / 1000);
> @@ -101,9 +103,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct
device *d,
>  	struct nouveau_drm *drm = nouveau_drm(dev);
>  	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  	long value;
> +	int ret;
>  
> -	if (kstrtol(buf, 10, &value) == -EINVAL)
> -		return count;
> +	ret = kstrtol(buf, 10, &value);
> +	if (ret)
> +		return ret;
>  
>  	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST_HYST,
>  			value / 1000);
> @@ -156,8 +160,9 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct
device_attribute *a,
>  	long value;
>  	int ret;
>  
> -	if (kstrtol(buf, 10, &value) == -EINVAL)
> -		return -EINVAL;
> +	ret = kstrtol(buf, 10, &value);
> +	if (ret)
> +		return ret;
>  
>  	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MIN_DUTY, value);
>  	if (ret < 0)
> @@ -179,8 +184,9 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct
device_attribute *a,
>  	long value;
>  	int ret;
>  
> -	if (kstrtol(buf, 10, &value) == -EINVAL)
> -		return -EINVAL;
> +	ret = kstrtol(buf, 10, &value);
> +	if (ret)
> +		return ret;
>  
>  	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MAX_DUTY, value);
>  	if (ret < 0)
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20170722/4abd24fb/attachment.sig>

Pierre Moreau

2017-Dec-29 21:56 UTC

head link

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

Ping: this can still be an issue today as kstrtol() & co haven’t changed
their
possible return value since the patch was written.

On 2017-07-22 — 14:13, Pierre Moreau wrote:> Reviewed-by: Pierre Moreau <pierre.morrow at free.fr>
> 
> On 2017-07-17 — 11:17, Dan Carpenter wrote:
> > kstrtol() and friends can return -EINVAL or -ERANGE.  We have to test
> > for both, otherwise the value is possibly uninitialized.  Also in some
> > of these files we accidentally return "count" on error
instead of a
> > negative error code.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > index 7c965648df80..5e75af91c446 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > @@ -68,9 +68,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct
device *d,
> >  	struct nouveau_drm *drm = nouveau_drm(dev);
> >  	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> >  	long value;
> > +	int ret;
> >  
> > -	if (kstrtol(buf, 10, &value) == -EINVAL)
> > -		return count;
> > +	ret = kstrtol(buf, 10, &value);
> > +	if (ret)
> > +		return ret;
> >  
> >  	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST,
> >  			value / 1000);
> > @@ -101,9 +103,11 @@
nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct device *d,
> >  	struct nouveau_drm *drm = nouveau_drm(dev);
> >  	struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> >  	long value;
> > +	int ret;
> >  
> > -	if (kstrtol(buf, 10, &value) == -EINVAL)
> > -		return count;
> > +	ret = kstrtol(buf, 10, &value);
> > +	if (ret)
> > +		return ret;
> >  
> >  	therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST_HYST,
> >  			value / 1000);
> > @@ -156,8 +160,9 @@ nouveau_hwmon_set_pwm1_min(struct device *d,
struct device_attribute *a,
> >  	long value;
> >  	int ret;
> >  
> > -	if (kstrtol(buf, 10, &value) == -EINVAL)
> > -		return -EINVAL;
> > +	ret = kstrtol(buf, 10, &value);
> > +	if (ret)
> > +		return ret;
> >  
> >  	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MIN_DUTY,
value);
> >  	if (ret < 0)
> > @@ -179,8 +184,9 @@ nouveau_hwmon_set_pwm1_max(struct device *d,
struct device_attribute *a,
> >  	long value;
> >  	int ret;
> >  
> > -	if (kstrtol(buf, 10, &value) == -EINVAL)
> > -		return -EINVAL;
> > +	ret = kstrtol(buf, 10, &value);
> > +	if (ret)
> > +		return ret;
> >  
> >  	ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MAX_DUTY,
value);
> >  	if (ret < 0)
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau

> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20171229/12666b93/attachment.sig>

Apparently Analagous Threads

Search for more possibly parallel threads

Nouveau - Jul 2017 - [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

[Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

Apparently Analagous Threads