Messages in this thread Patch in this message |  | | From | Arnd Bergmann <> | Date | Sun, 28 Jul 2019 17:27:10 +0200 | Subject | Re: [5.2 REGRESSION] Generic vDSO breaks seccomp-enabled userspace on i386 |
| |
On Sun, Jul 28, 2019 at 12:30 PM Thomas Gleixner <tglx@linutronix.de> wrote: > On Sun, 28 Jul 2019, Arnd Bergmann wrote: > > On Sat, Jul 27, 2019 at 7:53 PM Andy Lutomirski <luto@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
|  |