Re: [5.2 REGRESSION] Generic vDSO breaks seccomp-enabled userspace on i386

[Thomas Gleixner <tglx@linutronix.de>]

On Sun, 28 Jul 2019, Arnd Bergmann wrote:
> On Sat, Jul 27, 2019 at 7:53 PM Andy Lutomirski <luto@kernel.org> wrote:
> lib/vdso/gettimeofday.c:
> static __maybe_unused int
> __cvdso_clock_gettime32(clockid_t clock, struct old_timespec32 *res)
> {
>         struct __kernel_timespec ts;
>         int ret;
> 
>         if (res == NULL)
>                 goto fallback;
> 
>         ret = __cvdso_clock_gettime(clock, &ts);
> 
>         if (ret == 0) {
>                 res->tv_sec = ts.tv_sec;
>                 res->tv_nsec = ts.tv_nsec;
>         }
> 
>         return ret;
> 
> fallback:
>         return clock_gettime_fallback(clock, (struct __kernel_timespec *)res);
> }
> 
> So we get an 'old_timespec32' pointer from user space, and cast
> it to __kernel_timespec in order to pass it to the low-level function
> that actually fills in the 64-bit structure.
>
> On a little-endian machine, the first four bytes are actually correct
> here, but this is followed by tv_nsec=0 and 8 more bytes that overwrite
> whatever comes after the user space 'timespec'. [I missed the
> typecast as an indication of a bug during my review, sorry about
> that].

Which is totally irrelevant because res is NULL and that NULL pointer check
should simply return -EFAULT, which is what the syscall fallback returns
because the pointer is NULL.

But that NULL pointer check is inconsistent anyway:

 - 64 bit does not have it and never had

 - the vdso is not capable of handling faults properly anyway. If the
   pointer is not valid, then it will segfault. So just preventing the
   segfault for NULL is silly.

I'm going to just remove it.

Thanks,

	tglx