On Mon, Dec 30, 2019 at 1:27 PM Arnd Bergmann [off-list ref] wrote:

quoted

On Mon, Dec 23, 2019 at 3:31 PM Christophe Leroy [off-list ref] wrote:

quoted

+static __always_inline
+long clock_getres32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
+{
+       struct __kernel_timespec ts;
+       int ret = clock_getres_fallback(clock, &ts);
+
+       if (likely(!ret && _ts)) {
+               _ts->tv_sec = ts.tv_sec;
+               _ts->tv_nsec = ts.tv_nsec;
+       }
+       return ret;
+}

Please change these to call __NR_clock_gettime and __NR_clock_getres_time
instead of __NR_clock_gettime64/__NR_clock_getres_time64 for multiple reasons.

- When doing migration between containers, the vdso may get copied into
   an application running on a kernel that does not support the time64
   variants, and then the fallback fails.

- When CONFIG_COMPAT_32BIT_TIME is disabled, the time32 syscalls
   return -ENOSYS, and the vdso version should have the exact same behavior
   to avoid surprises. In particular an application that checks clock_gettime()
   to see if the time32 are in part of the kernel would get an incorrect result
   here.

arch/arm64/include/asm/vdso/compat_gettimeofday.h already does this,
I think you can just copy the implementation or find a way to share it.

There was a related discussion on this after a vdso regression on mips,
and I suggested to drop the time32 functions completely from the
vdso when CONFIG_COMPAT_32BIT_TIME is disabled, such as

diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S

b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 00c025ba4a92..605f259fa24c 100644

--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S

@@ -145,10 +145,12 @@ VERSION

                 __kernel_get_syscall_map;
  #ifndef CONFIG_PPC_BOOK3S_601
+#ifdef CONFIG_COMPAT_32BIT_TIME
                 __kernel_gettimeofday;
                 __kernel_clock_gettime;
                 __kernel_clock_getres;
                 __kernel_time;
+#endif
                 __kernel_get_tbfreq;
  #endif
                 __kernel_sync_dicache;

Any opinions on this? If everyone agrees with that approach, I can
send a cross-architecture patch to do this everywhere. It's probably
best though if Christophe adds that to his series as it touches a lot
of the same files and I would prefer to avoid conflicting changes.