Re: [PATCH v2 4/6] linux: Use long time_t __getitimer/__setitimer

[Alistair Francis <alistair23 at gmail dot com>]

On Tue, Feb 11, 2020 at 12:02 PM Vineet Gupta <vineetg76@gmail.com> wrote:
>
> Hi Alistair,
>
> On 2/10/20 9:43 AM, Alistair Francis wrote:
> > The Linux kernel expects itimerval to use a 32-bit time_t, even on archs
> > with a 64-bit time_t (like RV32). To address this let's convert
> > itimerval to/from 32-bit and 64-bit to ensure the kernel always gets
> > a 32-bit time_t.
> >
> > While we are converting these functions let's also convert them to be
> > the y2038 safe versions. This means there is a *64 function that is
> > called by a backwards compatible wrapper.
> > ---
>
> > +
> > +int
> > +__setitimer64 (__itimer_which_t which,
> > +               const struct __itimerval64 *restrict new_value,
> > +               struct __itimerval64 *restrict old_value)
> > +{
> > +  struct __itimerval32 new_value_32;
> > +
> > +  if (! in_time_t_range (new_value->it_interval.tv_sec))
> > +  {
> > +    __set_errno (EOVERFLOW);
> > +    return -1;
> > +  }
> > +  new_value_32.it_interval
> > +    = valid_timeval64_to_timeval32 (new_value->it_interval);
> > +
> > +  if (! in_time_t_range (new_value->it_value.tv_sec))
> > +  {
> > +    __set_errno (EOVERFLOW);
> > +    return -1;
> > +  }
> > +  new_value_32.it_value
> > +    = valid_timeval64_to_timeval32 (new_value->it_value);
> > +
> > +  if (old_value == NULL)
> > +    return INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, NULL);
> > +
> > +  struct __itimerval32 old_value_32;
> > +  if (INLINE_SYSCALL_CALL (setitimer, which, &new_value_32, &old_value_32) == -1)
> > +    return -1;
> > +
> > +  /* Write all fields of 'old_value' regardless of overflow.  */
> > +  old_value->it_interval
> > +     = valid_timeval32_to_timeval64 (old_value_32.it_interval);
> > +  old_value->it_value
> > +     = valid_timeval32_to_timeval64 (old_value_32.it_value);
> > +  return 0;
> > +}
> > +
> > +#if __TIMESIZE != 64
> > +int
> > +__setitimer (__itimer_which_t which,
> > +             const struct itimerval *restrict new_value,
> > +             struct itimerval *restrict old_value)
> > +{
> > +  int ret;
> > +  struct __itimerval64 new64, old64;
> > +
> > +  new64.it_interval
> > +    = valid_timeval_to_timeval64 (new_value->it_interval);
> > +  new64.it_value
> > +    = valid_timeval_to_timeval64 (new_value->it_value);
> > +
> > +  ret = __setitimer64 (which, &new64, &old64);
> > +
> > +  if (ret != 0)
> > +    return ret;
>
> I tested ARC port over your v1 next branch and it works fine in general. I still
> had 32-bit time_t so you have some more test coverage ;-)
>
> The glibc testsuite had some new failures, some of them are coming from the
> unchecked @old_value dereference (which would not hit for 64-bit time_t).
>
> Care to fix it please.

Fixed! Thanks for testing!

Alistair

>
> > +
> > +  old_value->it_interval
> > +    = valid_timeval64_to_timeval (old64.it_interval);
> > +  old_value->it_value
> > +    = valid_timeval64_to_timeval (old64.it_value);
> > +
> > +  return ret;
> > +}
> > +#endif
> > +weak_alias (__setitimer, setitimer)
> Thx,
> -Vineet