Re: [PATCH v2] posix-cpu-timers: Avoid undefined behaviour in timespec64_to_ns()

[Arnd Bergmann]

On Thu, Feb 28, 2019 at 5:25 AM Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote:
>
> On Tue, Feb 26, 2019 at 11:52 PM Xiongfeng Wang
> <wangxiongfeng2@xxxxxxxxxx> wrote:
> >
> > +++ b/kernel/time/posix-timers.c
> > @@ -853,8 +853,8 @@ static int do_timer_settime(timer_t timer_id, int flags,
> >         unsigned long flag;
> >         int error = 0;
> >
> > -       if (!timespec64_valid(&new_spec64->it_interval) ||
> > -           !timespec64_valid(&new_spec64->it_value))
> > +       if (!timespec64_valid_strict(&new_spec64->it_interval) ||
> > +           !timespec64_valid_strict(&new_spec64->it_value))
> >                 return -EINVAL;
> >
> >         if (old_spec64)
>
> sys_timer_settime() is a POSIX interface:
> http://pubs.opengroup.org/onlinepubs/7908799/xsh/timer_settime.html
>
> The timer_settime() function will fail if:
>
> [EINVAL] The timerid argument does not correspond to an id returned by
> timer_create() but not yet deleted by timer_delete().
>
> [EINVAL] A value structure specified a nanosecond value less than zero
> or greater than or equal to 1000 million.
>
> So we cannot return EINVAL here if we want to maintain POSIX compatibility.
> Maybe we should check for limit and saturate here at the syscall interface?

I think returning EINVAL here is better than silently truncating, we
just need to
document it in the Linux man page.
Note that truncation would set the time to just before the overflow,
it bad things
start to happen the instant after it returns from the kernel. This is possibly
worse than setting a random value that may or may not crash the system.

      Arnd