On Sun, Jul 28, 2019 at 12:30 PM Thomas Gleixner <t...@linutronix.de> wrote: > On Sun, 28 Jul 2019, Arnd Bergmann wrote: > > On Sat, Jul 27, 2019 at 7:53 PM Andy Lutomirski <l...@kernel.org> wrote:
> 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. Ah, you are right, I misread. Anyway, if we want to keep the traditional behavior and get fewer surprises for users of seccomp and anything else that might observe clock_gettime behavior, below is how I'd do it. (not even build tested, x86-only. I'll send a proper patch if this is where we want to take it). Arnd diff --git a/arch/x86/include/asm/vdso/gettimeofday.h b/arch/x86/include/asm/vdso/gettimeofday.h index ae91429129a6..f7b6a50d4811 100644 --- a/arch/x86/include/asm/vdso/gettimeofday.h +++ b/arch/x86/include/asm/vdso/gettimeofday.h @@ -70,6 +70,13 @@ long clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) return ret; } + +static __always_inline +long clock_gettime32_fallback(clockid_t _clkid, struct old_timespec32 *_ts) +{ + return -ENOSYS; +} + static __always_inline long gettimeofday_fallback(struct __kernel_old_timeval *_tv, struct timezone *_tz) @@ -97,7 +104,7 @@ long clock_getres_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) #else static __always_inline -long clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) +long clock_gettime_fallback(clockid_t _clkid, struct old_timespec32 *_ts) { long ret; @@ -113,6 +120,23 @@ long clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) return ret; } +static __always_inline +long clock_gettime32_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) +{ + long ret; + + asm ( + "mov %%ebx, %%edx \n" + "mov %[clock], %%ebx \n" + "call __kernel_vsyscall \n" + "mov %%edx, %%ebx \n" + : "=a" (ret), "=m" (*_ts) + : "0" (__NR_clock_gettime), [clock] "g" (_clkid), "c" (_ts) + : "edx"); + + return ret; +} + static __always_inline long gettimeofday_fallback(struct __kernel_old_timeval *_tv, struct timezone *_tz) diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c index 2d1c1f241fd9..3b3dab53d611 100644 --- a/lib/vdso/gettimeofday.c +++ b/lib/vdso/gettimeofday.c @@ -18,6 +18,7 @@ * clock_mode. * - gettimeofday_fallback(): fallback for gettimeofday. * - clock_gettime_fallback(): fallback for clock_gettime. + * - clock_gettime_fallback(): fallback for clock_gettime32. * - clock_getres_fallback(): fallback for clock_getres. */ #ifdef ENABLE_COMPAT_VDSO @@ -51,7 +52,7 @@ static int do_hres(const struct vdso_data *vd, clockid_t clk, ns = vdso_ts->nsec; last = vd->cycle_last; if (unlikely((s64)cycles < 0)) - return clock_gettime_fallback(clk, ts); + return -ENOSYS; ns += vdso_calc_delta(cycles, last, vd->mask, vd->mult); ns >>= vd->shift; @@ -81,15 +82,15 @@ static void do_coarse(const struct vdso_data *vd, clockid_t clk, } while (unlikely(vdso_read_retry(vd, seq))); } -static __maybe_unused int -__cvdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts) +static int +__cvdso_clock_gettime_common(clockid_t clock, struct __kernel_timespec *ts) { const struct vdso_data *vd = __arch_get_vdso_data(); u32 msk; /* Check for negative values or invalid clocks */ if (unlikely((u32) clock >= MAX_CLOCKS)) - goto fallback; + return -EINVAL; /* * Convert the clockid to a bitmask and use it to check which @@ -105,7 +106,15 @@ __cvdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts) return do_hres(&vd[CS_RAW], clock, ts); } -fallback: + return -EINVAL; +} + +static __maybe_unused int +__cvdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts) +{ + if (!__cvdso_clock_gettime_common(clock, ts)) + return 0; + return clock_gettime_fallback(clock, ts); } @@ -113,22 +122,14 @@ 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) { + if (!__cvdso_clock_gettime_common(clock, &ts)) { res->tv_sec = ts.tv_sec; res->tv_nsec = ts.tv_nsec; + return 0; } - return ret; - -fallback: - return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); + return clock_gettime32_fallback(clock, res); } static __maybe_unused int