Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1551253922-3307-1-git-send-email-wangxiongfeng2@huawei.com> <CABeXuvoJ3oKRp0z=CO_8kPhZ+-AHGwTaDBqzK2FuC_v5t6kU=A@mail.gmail.com>
In-Reply-To: <CABeXuvoJ3oKRp0z=CO_8kPhZ+-AHGwTaDBqzK2FuC_v5t6kU=A@mail.gmail.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 28 Feb 2019 09:44:32 +0100
Message-ID: <CAK8P3a0ip8+UaqnjGsC8GVY8CSEtccB4uUB-b5Zk1nrVqcY0eQ@mail.gmail.com>
Subject: Re: [PATCH v2] posix-cpu-timers: Avoid undefined behaviour in timespec64_to_ns()
To:     Deepa Dinamani <deepa.kernel@gmail.com>
Cc:     Xiongfeng Wang <wangxiongfeng2@huawei.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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