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

[H. Nikolaus Schaller]

Nobody with an idea?

> Am 17.11.2019 um 14:14 schrieb H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>:
> 
> 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
> 
> _______________________________________________
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> Letux-kernel@xxxxxxxxxxxxxxx
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel