linux-kernel - Re: [PATCH] time: copy tai value (International Atomic Time, in seconds) to output __user struct in get_old

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]

Message-ID: <CANDhNCqzLxsAx=XRBdL554SDZ83vQonnppTQPKgB6=RDhw_XjA@mail.gmail.com>
Date:   Wed, 23 Nov 2022 12:29:05 -0800
From:   John Stultz <jstultz@...gle.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Jacob Macneal <jake.macneal@...il.com>,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>
Subject: Re: [PATCH] time: copy tai value (International Atomic Time, in
 seconds) to output __user struct in get_old_timex32().

On Wed, Nov 23, 2022 at 11:53 AM Arnd Bergmann <arnd@...db.de> wrote:
> On Wed, Nov 23, 2022, at 19:54, John Stultz wrote:
> > On Sun, Nov 20, 2022 at 9:54 PM Jacob Macneal <jake.macneal@...il.com> wrote:
> >> --- a/kernel/time/time.c
> >> +++ b/kernel/time/time.c
> >> @@ -311,6 +311,7 @@ int get_old_timex32(struct __kernel_timex *txc, const struct old_timex32 __user
> >>         txc->calcnt = tx32.calcnt;
> >>         txc->errcnt = tx32.errcnt;
> >>         txc->stbcnt = tx32.stbcnt;
> >> +       txc->tai = tx32.tai;
> >>
> >
> > This does seem like something that was overlooked.
> >
> > Arnd: There isn't something more subtle I'm missing here, right?
>
> I agree. Looking at the git history, it seems that the tai field
> was added a long time ago in 153b5d054ac2 ("ntp: support for TAI").
> The commit correctly did the conversion for copying the data out
> of the kernel and did not copy the value in because it wasn't
> needed at the time.
>
> I don't see any user of the tai field that gets copied into
> the kernel, so the bug appears harmless, but Jacob's fix is

Oh, right. There is a quirk of the adjtimex ADJ_TAI interface (added
in 153b5d054ac2) where it for some reason used the constant field
instead of the newly added tai field.
So we never should be using the tai field value from userspace (only
writing it out), which might have been the reason it was not copied
over.

> nevertheless correct, as we should not use any uninitialized
> data in a structure that comes from userspace.

Agreed.

thanks
-john