Hi Dan-san,

Thank you so much for the fixes.
But very sorry for the problem caused by my changes.

I have reviewed them and those look good.

Reviewed-by: Tokunori Ikegami <ikeg...@allied-telesis.co.jp>

Regards,
Ikegami

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Tuesday, August 14, 2018 6:13 PM
> To: Jean Delvare; IKEGAMI Tokunori
> Cc: Guenter Roeck; linux-hwmon@vger.kernel.org;
> kernel-janit...@vger.kernel.org
> Subject: [PATCH 1/2] hwmon: (adt7475) Potential error pointer
> dereferences
> 
> The adt7475_update_device() function returns error pointers.  The
> problem is that in show_pwmfreq() we dereference it before the check.
> And then in pwm_use_point2_pwm_at_crit_show() there isn't a check at
> all.  I don't know if it's required, but it silences a static checker
> warning and it's doesn't hurt anything to check.
> 
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> 
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 90837f7c7d0f..16045149f3db 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -962,13 +962,14 @@ static ssize_t show_pwmfreq(struct device *dev,
> struct device_attribute *attr,
>  {
>       struct adt7475_data *data = adt7475_update_device(dev);
>       struct sensor_device_attribute_2 *sattr =
> to_sensor_dev_attr_2(attr);
> -     int i = clamp_val(data->range[sattr->index] & 0xf, 0,
> -                       ARRAY_SIZE(pwmfreq_table) - 1);
> +     int idx;
> 
>       if (IS_ERR(data))
>               return PTR_ERR(data);
> +     idx = clamp_val(data->range[sattr->index] & 0xf, 0,
> +                     ARRAY_SIZE(pwmfreq_table) - 1);
> 
> -     return sprintf(buf, "%d\n", pwmfreq_table[i]);
> +     return sprintf(buf, "%d\n", pwmfreq_table[idx]);
>  }
> 
>  static ssize_t set_pwmfreq(struct device *dev, struct device_attribute
> *attr,
> @@ -1004,6 +1005,10 @@ static ssize_t
> pwm_use_point2_pwm_at_crit_show(struct device *dev,
>                                       char *buf)
>  {
>       struct adt7475_data *data = adt7475_update_device(dev);
> +
> +     if (IS_ERR(data))
> +             return PTR_ERR(data);
> +
>       return sprintf(buf, "%d\n", !!(data->config4 &
> CONFIG4_MAXDUTY));
>  }
> 

Reply via email to