linux-kernel - Re: [PATCH 17/23] y2038: time: avoid timespec usage in settimeofday()

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 15 Nov 2019 14:50:42 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Abel Vesa <abelvesa@...ux.com>,
        y2038 Mailman List <y2038@...ts.linaro.org>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        David Howells <dhowells@...hat.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Christian Brauner <christian@...uner.io>,
        Jens Axboe <axboe@...nel.dk>, Ingo Molnar <mingo@...nel.org>,
        Corey Minyard <cminyard@...sta.com>,
        zhengbin <zhengbin13@...wei.com>,
        Li RongQing <lirongqing@...du.com>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH 17/23] y2038: time: avoid timespec usage in settimeofday()

On Fri, Nov 15, 2019 at 11:27 AM Rasmus Villemoes
<linux@...musvillemoes.dk> wrote:
> On 15/11/2019 08.58, Arnd Bergmann wrote:
> > On Fri, Nov 15, 2019 at 12:01 AM Abel Vesa <abelvesa@...ux.com> wrote:
> >>
> > --- a/kernel/time/time.c
> > +++ b/kernel/time/time.c
> > @@ -207,7 +207,7 @@ SYSCALL_DEFINE2(settimeofday, struct
> > __kernel_old_timeval __user *, tv,
> >                     get_user(new_ts.tv_nsec, &tv->tv_usec))
> >                         return -EFAULT;
> >
> > -               if (tv->tv_usec > USEC_PER_SEC)
> > +               if (new_ts->tv_usec > USEC_PER_SEC)
> >                         return -EINVAL;
>
> Hopefully not :)

No, I misquoted from a fix that I had temporarily applied, not the
version in linux-next.

>
> >                 new_ts.tv_nsec *= NSEC_PER_USEC;
>
> So the actual patch in next-20191115 does
>
> -               if (copy_from_user(&user_tv, tv, sizeof(*tv)))
> +               if (get_user(new_ts.tv_sec, &tv->tv_sec) ||
> +                   get_user(new_ts.tv_nsec, &tv->tv_usec))
>                         return -EFAULT;
>
> -               if (!timeval_valid(&user_tv))
> +               if (new_ts.tv_nsec > USEC_PER_SEC)
>                         return -EINVAL;
>
> -               new_ts.tv_sec = user_tv.tv_sec;
> -               new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
> +               new_ts.tv_nsec *= NSEC_PER_USEC;
>
> But removing the "user value is < 0" check, relying on the timespec
> value being rejected later, is wrong

You are right of course, so many ways to get this one line wrong...
Pushed more more update to the branch now.

Thanks for the careful review!

        Arnd