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: AS17314 8.43.84.0/22 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 [8.43.85.97]) (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 7FDAA1F8C6 for ; Sat, 10 Jul 2021 20:30:40 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5EC8D3847828 for ; Sat, 10 Jul 2021 20:30:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5EC8D3847828 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1625949039; bh=6f7yKGSj3Wm+PM35CwBpN4dtGLqqRGaaqKlhrRI5jpc=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=XwyB7ndkRsCUXFuzOKH07mQB75ggVAZJaWpnRRYRUrRpMUzJuy+DOaXz/S6MjgKn4 SW8D/19D0SE+AJraBDZrOk/pb7BKNyaX0Kz+kIorZshey4BPVNPeO0j9Mb3/lT8xvp O5G7ItaQh//InCtjvs4xFQ9gBWFzmvS6Ie9PVvQQ= Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id ECCCD385C40F for ; Sat, 10 Jul 2021 20:30:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ECCCD385C40F Received: by mail-pl1-x629.google.com with SMTP id u3so1777087plf.5 for ; Sat, 10 Jul 2021 13:30:13 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=6f7yKGSj3Wm+PM35CwBpN4dtGLqqRGaaqKlhrRI5jpc=; b=I2dIHiqnzM8jPYTjYoGABOmMmxrQTjST4PfFhgoqVvC3/jKYh/fx61wf1acHSIUBxL gQojJDIpUbpEnYXcQB5qSfIdwcj/BhFjdA15Q84q6tzHB5ywic+X2gJxhYofZQ3y7W+f T0r/You8bostYOVr9b6pHgWbkU0otZxtOLH1bt1iicy+rWpeXFic/rk6vESxgV29YCsD nHMkT5Td3jEctZmNzyfyhrpDB5+QUmlcE7Gx2UOtSvmFKtzGXiSOl+k0VHcLJ5CkXbO/ obIEXBqltwCupUzQ9X9TXeOWpdErC3kikbd1Gzy+4U/RREYNjVDcPTUcxtigOkPRicLi snFA== X-Gm-Message-State: AOAM533XPpF5hh5/EVr4rx5DbKtcwYxRLZe9mqmrlmHOB9jjs/tQb+j6 YJkerBA3jfngQavrOkUgtBtJISssCsEyRA== X-Google-Smtp-Source: ABdhPJwtFQ+dxv8KK37G4JX2ya2BsFz9G/z3C/FdtmbvAFzVdd6RcduEng8mrbNvKn43qs06z6oXIg== X-Received: by 2002:a17:90b:17d0:: with SMTP id me16mr5984643pjb.54.1625949012466; Sat, 10 Jul 2021 13:30:12 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id h9sm12261925pgi.43.2021.07.10.13.30.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Jul 2021 13:30:11 -0700 (PDT) Subject: Re: [PATCH] Linux: Use 32-bit vDSO for clock_gettime, gettimeofday, time (bug 28071) To: Florian Weimer References: <87czrqf5un.fsf@oldenburg.str.redhat.com> <878s2ef19p.fsf@oldenburg.str.redhat.com> <874kd2ey6s.fsf@oldenburg.str.redhat.com> Message-ID: <158cdbcb-5335-9ff4-cf3e-a45d8603d029@linaro.org> Date: Sat, 10 Jul 2021 17:30:08 -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: <874kd2ey6s.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 Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 10/07/2021 17:00, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 10/07/2021 15:54, Florian Weimer wrote: >>> * Adhemerval Zanella: >>> >>>> 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: >>> >>> But for __ASSUME_TIME64_SYSCALLS, clock_gettime64 will not fail. >>> >>> What am I missing? Is the issue that INLINE_VSYSCALL may set ENOSYS >>> artificially? >> >> I meant for __clock_gettime64, where it may still clobber the errno >> on older kernels (although it might be a fringe case). In any case, >> I still think making all time32 call to call time64 is a better >> implementation than add some specific calls for time32. > > So do you want to send an alternative patch? I can add my tests on top > of that. Could you check the following? I checked on a 5.11 kernel (64-bit vDSO), 4.4 kernel (32-bit vDSO without CLOCK_MONOTONIC support) and on a 3.10 kernel (no vDSO support). Using the test (a slight modified one from the bug report): -- #include #include #include int main (void) { struct timespec ts; errno = 0; clock_gettime (CLOCK_REALTIME, &ts); printf ("errno = %m (%d)\n", errno); errno = 0; clock_gettime (CLOCK_MONOTONIC, &ts); printf ("errno = %m (%d)\n", errno); } -- I see no syscall on 5.11 kernel, only a clock_gettime (CLOCK_MONOTONIC) on the 4.4 and a clock_gettime_time64 plus a clock_gettime on the 3.10. --- [PATCH] Linux: Use 32-bit vDSO for clock_gettime, gettimeofday, time (BZ# 28071) 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). This patch fixes by open-code INLINE_VSYSCALL macro and replace all INLINE_SYSCALL_CALL with INTERNAL_SYSCALL_CALLS. Now for __clock_gettime64, the 64-bit vDSO is used and the 32-bit vDSO is tried before falling back to 64-bit syscalls. 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). All architectures that provides 32-bit vDSO (i386, mips, powerpc, s390) modulo sparc; but I am not sure if some kernels versions do provide only 32-bit vDSO while still providing 64-bit time_t syscall. Regardless, for such cases the 64-bit time_t syscall is used if the vDSO returns overflowed 32-bit time_t. Tested on i686-linux-gnu (with a time64 and non-time64 kernel), x86_64-linux-gnu. Built with build-many-glibcs.py. Co-authored-by: Florian Weimer --- sysdeps/unix/sysv/linux/Makefile | 6 ++ sysdeps/unix/sysv/linux/clock_gettime.c | 50 ++++++++++++---- .../sysv/linux/tst-clock_gettime-clobber.c | 57 +++++++++++++++++++ .../sysv/linux/tst-gettimeofday-clobber.c | 37 ++++++++++++ sysdeps/unix/sysv/linux/tst-time-clobber.c | 36 ++++++++++++ 5 files changed, 174 insertions(+), 12 deletions(-) create mode 100644 sysdeps/unix/sysv/linux/tst-clock_gettime-clobber.c create mode 100644 sysdeps/unix/sysv/linux/tst-gettimeofday-clobber.c create mode 100644 sysdeps/unix/sysv/linux/tst-time-clobber.c diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 7d43dc95f2..a4769aa9db 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -213,6 +213,12 @@ ifeq ($(subdir),time) sysdep_headers += sys/timex.h bits/timex.h sysdep_routines += ntp_gettime ntp_gettimex + +tests += \ + tst-clock_gettime-clobber \ + tst-gettimeofday-clobber \ + tst-time-clobber \ + # tests endif ifeq ($(subdir),signal) diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c index cfe9370455..59b146702f 100644 --- a/sysdeps/unix/sysv/linux/clock_gettime.c +++ b/sysdeps/unix/sysv/linux/clock_gettime.c @@ -35,27 +35,53 @@ __clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp) #endif #ifdef HAVE_CLOCK_GETTIME64_VSYSCALL - r = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); -#else - r = INLINE_SYSCALL_CALL (clock_gettime64, clock_id, tp); + 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 - if (r == 0 || errno != ENOSYS) - return r; +#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 && tp32.tv_sec > 0) + { + *tp = valid_timespec_to_timespec64 (tp32); + return 0; + } + else if (r != 0) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); + /* Fallback to syscall if time32 overflow. */ + } +#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; -# ifdef HAVE_CLOCK_GETTIME_VSYSCALL - r = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32); -# else - r = INLINE_SYSCALL_CALL (clock_gettime, clock_id, &tp32); -# endif + r = INTERNAL_SYSCALL_CALL (clock_gettime, clock_id, &tp32); if (r == 0) - *tp = valid_timespec_to_timespec64 (tp32); + { + *tp = valid_timespec_to_timespec64 (tp32); + return 0; + } #endif - return r; + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-r); } #if __TIMESIZE != 64 diff --git a/sysdeps/unix/sysv/linux/tst-clock_gettime-clobber.c b/sysdeps/unix/sysv/linux/tst-clock_gettime-clobber.c new file mode 100644 index 0000000000..81a32bd15a --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-clock_gettime-clobber.c @@ -0,0 +1,57 @@ +/* Check that clock_gettime does not clobber errno on success. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static void +test_clock (clockid_t clk) +{ + printf ("info: testing clock: %d\n", (int) clk); + + for (int original_errno = 0; original_errno < 2; ++original_errno) + { + errno = original_errno; + struct timespec ts; + if (clock_gettime (clk, &ts) == 0) + TEST_COMPARE (errno, original_errno); + } +} + +static int +do_test (void) +{ + test_clock (CLOCK_BOOTTIME); + test_clock (CLOCK_BOOTTIME_ALARM); + test_clock (CLOCK_MONOTONIC); + test_clock (CLOCK_MONOTONIC_COARSE); + test_clock (CLOCK_MONOTONIC_RAW); + test_clock (CLOCK_PROCESS_CPUTIME_ID); + test_clock (CLOCK_REALTIME); + test_clock (CLOCK_REALTIME_ALARM); + test_clock (CLOCK_REALTIME_COARSE); +#ifdef CLOCK_TAI + test_clock (CLOCK_TAI); +#endif + test_clock (CLOCK_THREAD_CPUTIME_ID); + return 0; +} + +#include diff --git a/sysdeps/unix/sysv/linux/tst-gettimeofday-clobber.c b/sysdeps/unix/sysv/linux/tst-gettimeofday-clobber.c new file mode 100644 index 0000000000..bb9bdb67b0 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-gettimeofday-clobber.c @@ -0,0 +1,37 @@ +/* Check that gettimeofday does not clobber errno. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static int +do_test (void) +{ + for (int original_errno = 0; original_errno < 2; ++original_errno) + { + errno = original_errno; + struct timeval tv; + gettimeofday (&tv, NULL); + TEST_COMPARE (errno, original_errno); + } + return 0; +} + +#include diff --git a/sysdeps/unix/sysv/linux/tst-time-clobber.c b/sysdeps/unix/sysv/linux/tst-time-clobber.c new file mode 100644 index 0000000000..ad83be9be8 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-time-clobber.c @@ -0,0 +1,36 @@ +/* Check that time does not clobber errno. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static int +do_test (void) +{ + for (int original_errno = 0; original_errno < 2; ++original_errno) + { + errno = original_errno; + time (NULL); + TEST_COMPARE (errno, original_errno); + } + return 0; +} + +#include