[bug report] iio: hrtimer: Allow sub Hz granularity

[bug report] iio: hrtimer: Allow sub Hz granularity

[Dan Carpenter]

Hello Gwendal Grignou,

The patch dafcf4ed8392: "iio: hrtimer: Allow sub Hz granularity" from
Feb 25, 2021, leads to the following static checker warning:

	drivers/iio/trigger/iio-trig-hrtimer.c:68 iio_hrtimer_store_sampling_frequency()
	warn: assigned value is less than 'u32max'

drivers/iio/trigger/iio-trig-hrtimer.c
    49  static
    50  ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
    51                                               struct device_attribute *attr,
    52                                               const char *buf, size_t len)
    53  {
    54          struct iio_trigger *trig = to_iio_trigger(dev);
    55          struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
    56          unsigned long long val;
    57          u64 period;
    58          int integer, fract, ret;
    59  
    60          ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
    61          if (ret)
    62                  return ret;
    63          if (integer < 0 || fract < 0)
    64                  return -ERANGE;
    65  
    66          val = fract + 1000 * integer;  /* mHz */
                      ^^^^^^^^^^^^^^^^^^^^^^
"fract" and "integer" are integers so the arithmatic will wrap instead
of going above UINT_MAX

    67  
    68          if (!val || val > UINT_MAX)
                            ^^^^^^^^^^^^^^
Unpossible!

    69                  return -EINVAL;
    70  
    71          info->sampling_frequency[0] = integer;  /* Hz */
    72          info->sampling_frequency[1] = fract * 1000;  /* uHz */
    73          period = PSEC_PER_SEC;
    74          do_div(period, val);
    75          info->period = period;  /* nS */
    76  
    77          return len;
    78  }

regards,
dan carpenter

Re: [bug report] iio: hrtimer: Allow sub Hz granularity

[Jonathan Cameron]

On Wed, 31 Mar 2021 17:50:37 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Hello Gwendal Grignou,
> 
> The patch dafcf4ed8392: "iio: hrtimer: Allow sub Hz granularity" from
> Feb 25, 2021, leads to the following static checker warning:
> 
> 	drivers/iio/trigger/iio-trig-hrtimer.c:68 iio_hrtimer_store_sampling_frequency()
> 	warn: assigned value is less than 'u32max'
Coverity + Gustavo spotted this one as well. I've just queued up a fix
based on 1000ULL to ensure the maths is done at appropriate precision.

Thanks,

Jonathan

> 
> drivers/iio/trigger/iio-trig-hrtimer.c
>     49  static
>     50  ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
>     51                                               struct device_attribute *attr,
>     52                                               const char *buf, size_t len)
>     53  {
>     54          struct iio_trigger *trig = to_iio_trigger(dev);
>     55          struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
>     56          unsigned long long val;
>     57          u64 period;
>     58          int integer, fract, ret;
>     59  
>     60          ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
>     61          if (ret)
>     62                  return ret;
>     63          if (integer < 0 || fract < 0)
>     64                  return -ERANGE;
>     65  
>     66          val = fract + 1000 * integer;  /* mHz */
>                       ^^^^^^^^^^^^^^^^^^^^^^
> "fract" and "integer" are integers so the arithmatic will wrap instead
> of going above UINT_MAX
> 
>     67  
>     68          if (!val || val > UINT_MAX)
>                             ^^^^^^^^^^^^^^
> Unpossible!
> 
>     69                  return -EINVAL;
>     70  
>     71          info->sampling_frequency[0] = integer;  /* Hz */
>     72          info->sampling_frequency[1] = fract * 1000;  /* uHz */
>     73          period = PSEC_PER_SEC;
>     74          do_div(period, val);
>     75          info->period = period;  /* nS */
>     76  
>     77          return len;
>     78  }
> 
> regards,
> dan carpenter