linux-kernel - Re: [PATCH v2] posix-cpu-timers: Avoid undefined behaviour in timespec64_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Feb 2019 09:44:32 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Deepa Dinamani <deepa.kernel@...il.com>
Cc:     Xiongfeng Wang <wangxiongfeng2@...wei.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] posix-cpu-timers: Avoid undefined behaviour in timespec64_to_ns()

On Thu, Feb 28, 2019 at 5:25 AM Deepa Dinamani <deepa.kernel@...il.com> wrote:
>
> On Tue, Feb 26, 2019 at 11:52 PM Xiongfeng Wang
> <wangxiongfeng2@...wei.com> 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