Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 07.11.2019 um 17:21 schrieb H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>:
> 
> Hi,
> I am trying to run v5.4-rc6 on the CI20 board (jz4780) and it
> is almost ok. Except one strange thing.
> 
> If I install a v4.19.81 kernel I can initialize the
> ethernet interface and dhclient works.
> 
> If I install a v5.4-rc6 kernel on exactly the same
> rootfs dhclient fails with
> 
> root@letux:~# dhclient
> ../../../../lib/isc/unix/time.c:200: Operation not permitted
> root@letux:~#
> 
> I have done some strace and the first significant difference
> is that with v5.4-rc6 there is no gettimeofday syscall.
> 
> Another symptom pointing in the same direction is that
> after manually assigning an IP address I can run ping
> but get strange time values.
> 
> So it may be that
> 
> 24640f233b46 mips: Add support for generic vDSO
> 
> did break gettimeofday when used with latest Debian Stretch
> libraries. I tried to git revert but there are conflicts.
> 
> Just a side-note: both kernels work with Debian Jessie,
> which likely has an older gettimeofday wrapper that
> is not influenced by some subtle change.

I finally found time to do a bisect and it confirms:

24640f233b466051ad3a5d2786d2951e43026c9d is the first bad commit
commit 24640f233b466051ad3a5d2786d2951e43026c9d
Author: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
Date:   Fri Jun 21 10:52:46 2019 +0100

    mips: Add support for generic vDSO
    
    The mips vDSO library requires some adaptations to take advantage of the
    newly introduced generic vDSO library.
    
    Introduce the following changes:
     - Modification of vdso.c to be compliant with the common vdso datapage
     - Use of lib/vdso for gettimeofday
    
    Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
    Cc: Paul Burton <paul.burton@xxxxxxxx>
    Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
    [paul.burton@xxxxxxxx: Prepend $(src) to config-n32-o32-env.c path.]
    Signed-off-by: Paul Burton <paul.burton@xxxxxxxx>

 arch/mips/Kconfig                         |   2 +
 arch/mips/include/asm/vdso.h              |  78 +--------------
 arch/mips/include/asm/vdso/gettimeofday.h | 151 ++++++++++++++++++++++++++++++
 arch/mips/include/asm/vdso/vdso.h         |  85 +++++++++++++++++
 arch/mips/include/asm/vdso/vsyscall.h     |  43 +++++++++
 arch/mips/kernel/vdso.c                   |  37 ++------
 arch/mips/vdso/Makefile                   |  33 ++++++-
 arch/mips/vdso/config-n32-o32-env.c       |  17 ++++
 arch/mips/vdso/elf.S                      |   2 +-
 arch/mips/vdso/sigreturn.S                |   2 +-
 arch/mips/vdso/vdso.h                     |  85 -----------------
 arch/mips/vdso/vgettimeofday.c            |  40 ++++++++
 12 files changed, 378 insertions(+), 197 deletions(-)
 create mode 100644 arch/mips/include/asm/vdso/gettimeofday.h
 create mode 100644 arch/mips/include/asm/vdso/vdso.h
 create mode 100644 arch/mips/include/asm/vdso/vsyscall.h
 create mode 100644 arch/mips/vdso/config-n32-o32-env.c
 delete mode 100644 arch/mips/vdso/vdso.h
 create mode 100644 arch/mips/vdso/vgettimeofday.c

So this patch intoruced in v5.4-rc1 breaks compatibility with Debian 9.9
user space assumptions.

One thing seems strange to me:

-/**
- * union mips_vdso_data - Data provided by the kernel for the VDSO.
- * @xtime_sec:         Current real time (seconds part).
- * @xtime_nsec:                Current real time (nanoseconds part, shifted).
- * @wall_to_mono_sec:  Wall-to-monotonic offset (seconds part).
- * @wall_to_mono_nsec: Wall-to-monotonic offset (nanoseconds part).
- * @seq_count:         Counter to synchronise updates (odd = updating).
- * @cs_shift:          Clocksource shift value.
- * @clock_mode:                Clocksource to use for time functions.
- * @cs_mult:           Clocksource multiplier value.
- * @cs_cycle_last:     Clock cycle value at last update.
- * @cs_mask:           Clocksource mask value.
- * @tz_minuteswest:    Minutes west of Greenwich (from timezone).
- * @tz_dsttime:                Type of DST correction (from timezone).
- *
- * This structure contains data needed by functions within the VDSO. It is
- * populated by the kernel and mapped read-only into user memory. The time
- * fields are mirrors of internal data from the timekeeping infrastructure.
- *
- * Note: Care should be taken when modifying as the layout must remain the same
- * for both 64- and 32-bit (for 32-bit userland on 64-bit kernel).
- */
 union mips_vdso_data {
-       struct {
-               u64 xtime_sec;
-               u64 xtime_nsec;
-               u64 wall_to_mono_sec;
-               u64 wall_to_mono_nsec;
-               u32 seq_count;
-               u32 cs_shift;
-               u8 clock_mode;
-               u32 cs_mult;
-               u64 cs_cycle_last;
-               u64 cs_mask;
-               s32 tz_minuteswest;
-               s32 tz_dsttime;
-       };
-
+       struct vdso_data data[CS_BASES];
        u8 page[PAGE_SIZE];
 };

If I look at the definition of vdso_data it *is* significantly differen
from mips_vdso_data.

What I would assume is that the struct mips_vdso_data is embossed in user
space code and therefore using vdso_data instead is breaking API.

Please advise what I should try or check to narrow down further.

BR and thanks,
Nikolaus Schaller