From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-4.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id EFDF51F8C6 for ; Sat, 10 Jul 2021 18:22:54 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CE1863864824 for ; Sat, 10 Jul 2021 18:22:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CE1863864824 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1625941369; bh=gfby3EqhnswgiR04RtTyAxkHGSNEWT3lWMIbBqCG+4A=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Bg6+SVhkAYgX/SYqIm8xo915Xhv6IXQHB7KbLPepz2fyuqwTvLN7txqJ5WDVwUEYM EQKBL/n49YmQkyd1v68ekoJcIuEdbaDnq1uTwAgLwdegytqxEZUbPsvz72L3srw8Al fFvTXpSFcrl8zZA46nMPOiRiRdb6RIAK8oGQpkbM= Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by sourceware.org (Postfix) with ESMTPS id 745F3385AC3F for ; Sat, 10 Jul 2021 18:22:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 745F3385AC3F Received: by mail-pf1-x42d.google.com with SMTP id o201so6902980pfd.1 for ; Sat, 10 Jul 2021 11:22:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gfby3EqhnswgiR04RtTyAxkHGSNEWT3lWMIbBqCG+4A=; b=iqsabjPbAoV6ZaXNtuVKNqSRWmiWGYed43kwqT1ldhF5u01st3OOYUU8mRicmJk7IF xWMqAvF+IF5SJs5kG+5R8U6wSvzokxsopG8qQWWuTykzSEhpQYJIqAzfEvxZhZfFwBLy B3uQP6bW3OFrx7YcwbfI9qIEU3KSIPbST8C2BJRzWe/GHXWE8vY6jZoJEMGZhfoRZoFR mgCyIXCJV/wxNcq29LUKUswWzDmNuzGRgva0Al08vmjWXAxswY+VDLUVExUz0NGQbVWN UUq8kz5g8Yt0crXkLxjWWhviBTklcHYLTQdZqHF4uKuEruxt7wCrJyQL9jRZdlMI2ZnV 8kfg== X-Gm-Message-State: AOAM533wt0DYymseQbUTO/7fJpTuP8r5J5eqMQCUBGLCW581iEtWd00X XQpPBKWzSX0OBnB0LHpjj9bTSLQ6isoP3w== X-Google-Smtp-Source: ABdhPJwu1dN1DG3GaMb49L0NegvTayof7qE/1+Gct4qJ3MozUXTLJKlhoyIJdQORvWIzO4Dr8YSu/g== X-Received: by 2002:a65:40c6:: with SMTP id u6mr45525746pgp.390.1625941344281; Sat, 10 Jul 2021 11:22:24 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id 1sm9966002pfm.123.2021.07.10.11.22.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Jul 2021 11:22:23 -0700 (PDT) Subject: Re: [PATCH] Linux: Use 32-bit vDSO for clock_gettime, gettimeofday, time (bug 28071) To: Florian Weimer , libc-alpha@sourceware.org References: <87czrqf5un.fsf@oldenburg.str.redhat.com> Message-ID: Date: Sat, 10 Jul 2021 15:22:20 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <87czrqf5un.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Adhemerval Zanella via Libc-alpha Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 10/07/2021 14:15, Florian Weimer via Libc-alpha wrote: > The previous approach defeats the vDSO optimization on older kernels > because a failing clock_gettime64 system call is performed on every > function call. It also results in a clobbered errno value, exposing > an OpenJDK bug (JDK-8270244). > > The existing layering is preserved for __ASSUME_TIME64_SYSCALLS. > Otherwise, 32-bit time and gettimeofday use 32-bit clock_gettime, > and clock_gettime is implemented using the vDSO. > > Tested on i686-linux-gnu (with a time64 and non-time64 kernel), > x86_64-linux-gnu. Built with build-many-glibcs.py. > > --- > sysdeps/unix/sysv/linux/Makefile | 9 +++- > sysdeps/unix/sysv/linux/clock_gettime.c | 8 +++ > sysdeps/unix/sysv/linux/gettimeofday.c | 21 +++++--- > sysdeps/unix/sysv/linux/time.c | 22 ++++++--- > .../unix/sysv/linux/tst-clock_gettime-clobber.c | 57 ++++++++++++++++++++++ > sysdeps/unix/sysv/linux/tst-gettimeofday-clobber.c | 37 ++++++++++++++ > sysdeps/unix/sysv/linux/tst-time-clobber.c | 36 ++++++++++++++ > 7 files changed, 173 insertions(+), 17 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 7d43dc95f2..9f00e53de4 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -213,7 +213,14 @@ ifeq ($(subdir),time) > sysdep_headers += sys/timex.h bits/timex.h > > sysdep_routines += ntp_gettime ntp_gettimex > -endif > + > +tests += \ > + tst-clock_gettime-clobber \ > + tst-gettimeofday-clobber \ > + tst-time-clobber \ > + # tests > + > +endif# $(subdir) == time > > ifeq ($(subdir),signal) > tests-special += $(objpfx)tst-signal-numbers.out > diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c > index cfe9370455..86a72ecf5a 100644 > --- a/sysdeps/unix/sysv/linux/clock_gettime.c > +++ b/sysdeps/unix/sysv/linux/clock_gettime.c > @@ -64,6 +64,7 @@ libc_hidden_def (__clock_gettime64) > int > __clock_gettime (clockid_t clock_id, struct timespec *tp) > { > +# ifdef __ASSUME_TIME64_SYSCALLS > int ret; > struct __timespec64 tp64; > > @@ -81,6 +82,13 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp) > } > > return ret; > +# else /* !__ASSUME_TIME64_SYSCALLS */ > +# ifdef HAVE_CLOCK_GETTIME_VSYSCALL > + return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); > +# else > + return INLINE_SYSCALL_CALL (clock_gettime, clock_id, tp); > +# endif > +# endif /* !__ASSUME_TIME64_SYSCALLS */ > } > #endif > libc_hidden_def (__clock_gettime) This does not fix the issue for __ASSUME_TIME64_SYSCALLS where it still uses INLINE_SYSCALL_CALL which might clobber the errno, besides adding another ifdef code path (which I really want to avoid). Instead I think we need to open-coded the INLINE_VSYSCALL macro and replace all INLINE_SYSCALL_CALL with INTERNAL_SYSCALL_CALLS: /* Get current value of CLOCK and store it in TP. */ int __clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp) { int r; #ifdef HAVE_CLOCK_GETTIME64_VSYSCALL int (*vdso_time64)(clockid_t clock_id, struct __timespec64 *tp) = GLRO(dl_vdso_clock_gettime64); if (vdso_time64 != NULL) { r = INTERNAL_VSYSCALL_CALL (vdso_time64, 2, clock_id, tp); if (r == 0) return 0; return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); } #endif #ifdef HAVE_CLOCK_GETTIME_VSYSCALL int (*vdso_time)(clockid_t clock_id, struct timespec *tp) = GLRO(dl_vdso_clock_gettime); if (vdso_time != NULL) { struct timespec tp32; r = INTERNAL_VSYSCALL_CALL (vdso_time, 2, clock_id, &tp32); if (r == 0) { *tp = valid_timespec_to_timespec64 (tp32); return 0; } return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); } #endif r = INTERNAL_SYSCALL_CALL (clock_gettime64, clock_id, tp); if (r == 0) return 0; if (-r != ENOSYS) return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); #ifndef __ASSUME_TIME64_SYSCALLS /* Fallback code that uses 32-bit support. */ struct timespec tp32; r = INTERNAL_SYSCALL_CALL (clock_gettime, clock_id, &tp32); if (r == 0) { *tp = valid_timespec_to_timespec64 (tp32); return 0; } #endif return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); } Keep in mind the previous code preferred 64-bit syscall for the case where the kernel provides 64-bit time_t syscalls *and* also a 32-bit vDSO (in this case the *64-bit* syscall should be preferable over the vDSO). I do not know if this is still the case for most of architectures, maybe we should ignore this scenario and assume that if the architecture provided a clock_gettime vDSO and ahs 64-bit support it would also provide a 64-bit vDSO clock_gettime. I have tested the above with the clobber tests on this patch on a kernel v5.11 (64-bit vDSO), v4.4 (32-bit vDSO), and 3.10 (no vDSO). > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c > index 4197be5656..1b353956ea 100644 > --- a/sysdeps/unix/sysv/linux/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > @@ -16,18 +16,17 @@ > License along with the GNU C Library; if not, see > . */ > > +#include > +#include > #include > #include > +#include > > /* Optimize the function call by setting the PLT directly to vDSO symbol. */ > #ifdef USE_IFUNC_GETTIMEOFDAY > # include > -# include > > # ifdef SHARED > -# include > -# include > - > static int > __gettimeofday_syscall (struct timeval *restrict tv, void *restrict tz) > { > @@ -54,7 +53,7 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz) > } > # endif > weak_alias (__gettimeofday, gettimeofday) > -#else /* USE_IFUNC_GETTIMEOFDAY */ > +#else /* !USE_IFUNC_GETTIMEOFDAY */ > /* Conversion of gettimeofday function to support 64 bit time on archs > with __WORDSIZE == 32 and __TIMESIZE == 32/64 */ > #include > @@ -73,9 +72,12 @@ __gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz) > return 0; > } > > -# if __TIMESIZE != 64 > +# if __TIMESIZE == 64 > +weak_alias (__gettimeofday, gettimeofday) > +# else /* __TIMESIZE != 64 */ > libc_hidden_def (__gettimeofday64) > > +# ifdef __ASSUME_TIME64_SYSCALL > int > __gettimeofday (struct timeval *restrict tv, void *restrict tz) > { > @@ -92,6 +94,9 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz) > *tv = valid_timeval64_to_timeval (tv64); > return 0; > } > -# endif > weak_alias (__gettimeofday, gettimeofday) > -#endif > +# else /* !__ASSUME_TIME64_SYSCALL */ > +# include