unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time
@ 2020-05-08 14:56 Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch series converts clock_adjtime to support 64 bit time (by using
clock_adjtime64) when __ASSUME_TIME64_SYSCALLS is defined.

This change required introduction of new internal type - struct
__ntptimeval64 to accomodate 64 bit time on architectures
with __TIMESIZE != 64 and __WORDSIZE == 32.
In several places in the glibc the struct timeval has been replaced with struct
__timeval64.

As a result new, 64 bit supporting functions:
__adjtime64
__adjtimex64
__ntp_gettime64
__ntp_gettimex64

were introduced.

Moreover, the clock_adjtime64 syscall handling in Linux kernel has been broken
up till v5.4.

Lukasz Majewski (7):
  y2038: linux: Provide __clock_adjtime64 implementation
  y2038: linux: Provide ___adjtimex64 implementation
  y2038: linux: Provide __adjtime64 implementation
  y2038: Introduce struct __ntptimeval64 - new internal glibc type
  y2038: Provide conversion helpers for struct __ntptimeval64
  y2038: linux: Provide __ntp_gettime64 implementation
  y2038: linux: Provide __ntp_gettimex64 implementation

 include/sys/time.h                          |  9 +++
 sysdeps/unix/sysv/linux/Makefile            |  2 +-
 sysdeps/unix/sysv/linux/adjtime.c           | 26 ++++++--
 sysdeps/unix/sysv/linux/adjtimex.c          | 21 +++++-
 sysdeps/unix/sysv/linux/clock_adjtime.c     | 71 +++++++++++++++++++++
 sysdeps/unix/sysv/linux/include/sys/timex.h | 63 ++++++++++++++++++
 sysdeps/unix/sysv/linux/ntp_gettime.c       | 24 ++++++-
 sysdeps/unix/sysv/linux/ntp_gettimex.c      | 24 ++++++-
 sysdeps/unix/sysv/linux/syscalls.list       |  1 -
 9 files changed, 227 insertions(+), 14 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c

-- 
2.20.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-15 10:03   ` Lukasz Majewski
  2020-05-19 19:07   ` Adhemerval Zanella via Libc-alpha
  2020-05-08 14:56 ` [PATCH v2 2/7] y2038: linux: Provide ___adjtimex64 implementation Lukasz Majewski
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch replaces auto generated wrapper (as described in
sysdeps/unix/sysv/linux/syscalls.list) for clock_adjtime with one which adds
extra support for reading 64 bit time values on machines with __TIMESIZE != 64.

To achieve this goal new __clock_adjtime64 explicit 64 bit function for
adjusting Linux clock has been added.
Moreover, a 32 bit version - __clock_adjtime has been refactored to internally
use __clock_adjtime64.

The __clock_adjtime is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversions between 64 bit
struct __timespec64 and struct timespec.

The new __clock_adjtime64 syscall available from Linux 5.1+ has been used, when
applicable.
Up till v5.4 in the Linux kernel there was a bug preventing this call from
obtaining correct struct's timex time.tv_sec time after time_t overflow
(i.e. not being Y2038 safe).

Build tests:
- ./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Linux kernel, headers and minimal kernel version for glibc build test matrix:
- Linux v5.1 (with clock_adjtime64) and glibc build with v5.1 as
  minimal kernel version (--enable-kernel="5.1.0")
  The __ASSUME_TIME64_SYSCALLS flag defined.

- Linux v5.1 and default minimal kernel version
  The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports clock_adjtime64
  syscall.

- Linux v4.19 (no clock_adjtime64 support) with default minimal kernel version
  for contemporary glibc (3.2.0)
  This kernel doesn't support clock_adjtime64 syscall, so the fallback to
  clock_adjtime is tested.

Above tests were performed with Y2038 redirection applied as well as without
(so the __TIMESIZE != 64 execution path is checked as well).

No regressions were observed.

Changes for v2:
- Fix wrong indentation
- Remove the check for (ret >= TIME_OK && ret <= TIME_ERROR) in the
  clock_adjtime64 call - just check if syscall returned ENOSYS
- Add 64 bit time overflow for ADJ_SETOFFSET set in modes
---
 sysdeps/unix/sysv/linux/Makefile            |  2 +-
 sysdeps/unix/sysv/linux/clock_adjtime.c     | 71 +++++++++++++++++++++
 sysdeps/unix/sysv/linux/include/sys/timex.h |  3 +
 sysdeps/unix/sysv/linux/syscalls.list       |  1 -
 4 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 0326f92c40..2f2fd5d4d0 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -60,7 +60,7 @@ sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
 		   personality epoll_wait tee vmsplice splice \
 		   open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \
 		   timerfd_gettime timerfd_settime prctl \
-		   process_vm_readv process_vm_writev
+		   process_vm_readv process_vm_writev clock_adjtime
 
 CFLAGS-gethostid.c = -fexceptions
 CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables
diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c b/sysdeps/unix/sysv/linux/clock_adjtime.c
new file mode 100644
index 0000000000..5c02031bb5
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/clock_adjtime.c
@@ -0,0 +1,71 @@
+/* clock_adjtime -- tune kernel clock
+   Copyright (C) 2020 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; see the file COPYING.LIB.  If
+   not, see <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sysdep.h>
+#include <sys/timex.h>
+#include <kernel-features.h>
+
+int
+__clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64)
+{
+#ifdef __ASSUME_TIME64_SYSCALLS
+# ifndef __NR_clock_adjtime64
+#  define __NR_clock_adjtime64 __NR_clock_adjtime
+# endif
+	return INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
+#else
+  int ret = INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
+  if (errno != ENOSYS)
+    return ret;
+
+  if (tx64->modes & ADJ_SETOFFSET
+      && ! in_time_t_range (tx64->time.tv_sec))
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+
+  struct timex tx32 = valid_timex64_to_timex (*tx64);
+  int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
+  *tx64 = valid_timex_to_timex64 (tx32);
+
+  return retval;
+#endif
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__clock_adjtime64)
+
+int
+__clock_adjtime (const clockid_t clock_id, struct timex *tx)
+{
+  struct __timex64 tx64;
+  int retval;
+
+  tx64 = valid_timex_to_timex64 (*tx);
+  retval = __clock_adjtime64 (clock_id, &tx64);
+  *tx = valid_timex64_to_timex (tx64);
+
+  return retval;
+}
+#endif
+libc_hidden_def (__clock_adjtime);
+strong_alias (__clock_adjtime, clock_adjtime)
diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index 3efe7cd306..2848c5cf76 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -30,6 +30,7 @@ libc_hidden_proto (__adjtimex)
 /* Local definition of 64 bit time supporting timex struct */
 #  if __TIMESIZE == 64
 #   define __timex64 timex
+#   define __clock_adjtime64 __clock_adjtime
 #  else
 
 struct __timex64
@@ -71,6 +72,8 @@ struct __timex64
   int  :32;
   int  :32;
 };
+extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
+libc_hidden_proto (__clock_adjtime64);
 #  endif
 
 /* Convert a known valid struct timex into a struct __timex64.  */
diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
index 1d8893d1b8..01ec2bfa95 100644
--- a/sysdeps/unix/sysv/linux/syscalls.list
+++ b/sysdeps/unix/sysv/linux/syscalls.list
@@ -4,7 +4,6 @@ alarm		-	alarm		i:i	alarm
 bdflush		EXTRA	bdflush		i:ii	__compat_bdflush	bdflush@GLIBC_2.0:GLIBC_2.23
 capget		EXTRA	capget		i:pp	capget
 capset		EXTRA	capset		i:pp	capset
-clock_adjtime	EXTRA	clock_adjtime	i:ip	__clock_adjtime		clock_adjtime
 create_module	EXTRA	create_module	3	__compat_create_module	create_module@GLIBC_2.0:GLIBC_2.23
 delete_module	EXTRA	delete_module	3	delete_module
 epoll_create	EXTRA	epoll_create	i:i	epoll_create
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/7] y2038: linux: Provide ___adjtimex64 implementation
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 3/7] y2038: linux: Provide __adjtime64 implementation Lukasz Majewski
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch provides new ___adjtimex64 explicit 64 bit function for adjusting
Linux kernel clock.

Internally, the __clock_adjtime64 syscall is used. This patch is necessary
for having architectures with __WORDSIZE == 32 Y2038 safe.

Moreover, a 32 bit version - ___adjtimex has been refactored to internally
use ___adjtimex64.

The ___adjtimex is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
timex and 64 bit struct __timex64.

Last but not least, in ___adjtimex64 function the __clock_adjtime syscall has
been replaced with __clock_adjtime64 to support 64 bit time on architectures
with __WORDSIZE == 32 and __TIMESIZE != 64.

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without to
test the proper usage of both ___adjtimex64 and ___adjtimex.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 sysdeps/unix/sysv/linux/adjtimex.c          | 21 +++++++++++++++++++--
 sysdeps/unix/sysv/linux/include/sys/timex.h |  3 +++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/adjtimex.c b/sysdeps/unix/sysv/linux/adjtimex.c
index ebc17476a7..683cc65696 100644
--- a/sysdeps/unix/sysv/linux/adjtimex.c
+++ b/sysdeps/unix/sysv/linux/adjtimex.c
@@ -20,11 +20,28 @@
 #include <sysdep.h>
 
 int
-___adjtimex (struct timex *buf)
+___adjtimex64 (struct __timex64 *tx64)
 {
-  return __clock_adjtime (CLOCK_REALTIME, buf);
+  return __clock_adjtime64 (CLOCK_REALTIME, tx64);
 }
 
+#if __TIMESIZE != 64
+libc_hidden_def (___adjtimex64)
+
+int
+___adjtimex (struct timex *tx)
+{
+  struct __timex64 tx64;
+  int retval;
+
+  tx64 = valid_timex_to_timex64 (*tx);
+  retval = ___adjtimex64 (&tx64);
+  *tx = valid_timex64_to_timex (tx64);
+
+  return retval;
+}
+#endif
+
 #ifdef VERSION_adjtimex
 weak_alias (___adjtimex, __wadjtimex);
 weak_alias (___adjtimex, __wnadjtime);
diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index 2848c5cf76..cf3059f63c 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -31,6 +31,7 @@ libc_hidden_proto (__adjtimex)
 #  if __TIMESIZE == 64
 #   define __timex64 timex
 #   define __clock_adjtime64 __clock_adjtime
+#   define ___adjtimex64 ___adjtimex
 #  else
 
 struct __timex64
@@ -74,6 +75,8 @@ struct __timex64
 };
 extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
 libc_hidden_proto (__clock_adjtime64);
+extern int ___adjtimex64 (struct __timex64 *tx64);
+libc_hidden_proto (___adjtimex64)
 #  endif
 
 /* Convert a known valid struct timex into a struct __timex64.  */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/7] y2038: linux: Provide __adjtime64 implementation
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 2/7] y2038: linux: Provide ___adjtimex64 implementation Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type Lukasz Majewski
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch provides new __adjtime64 explicit 64 bit function for adjusting
Linux kernel clock.

Internally, the __clock_adjtime64 syscall is used instead of __adjtimex. This
patch is necessary for having architectures with __WORDSIZE == 32 Y2038 safe.

Moreover, a 32 bit version - __adjtime has been refactored to internally use
__adjtime64.

The __adjtime is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
timeval and 64 bit struct __timeval64.

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without to
test the proper usage of both __adjtime64 and __adjtime.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 include/sys/time.h                |  9 +++++++++
 sysdeps/unix/sysv/linux/adjtime.c | 26 ++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/sys/time.h b/include/sys/time.h
index 8153d75033..567e4b7562 100644
--- a/include/sys/time.h
+++ b/include/sys/time.h
@@ -26,6 +26,15 @@ extern int __settimezone (const struct timezone *__tz)
 	attribute_hidden;
 extern int __adjtime (const struct timeval *__delta,
 		      struct timeval *__olddelta);
+
+#  include <struct___timeval64.h>
+#  if __TIMESIZE == 64
+#   define __adjtime64 __adjtime
+#  else
+extern int __adjtime64 (const struct __timeval64 *itv,
+                        struct __timeval64 *otv);
+libc_hidden_proto (__adjtime64)
+#  endif
 extern int __getitimer (enum __itimer_which __which,
 			struct itimerval *__value);
 extern int __setitimer (enum __itimer_which __which,
diff --git a/sysdeps/unix/sysv/linux/adjtime.c b/sysdeps/unix/sysv/linux/adjtime.c
index c142f4f6ea..3f9a4ea2eb 100644
--- a/sysdeps/unix/sysv/linux/adjtime.c
+++ b/sysdeps/unix/sysv/linux/adjtime.c
@@ -24,13 +24,13 @@
 #define MIN_SEC	(INT_MIN / 1000000L + 2)
 
 int
-__adjtime (const struct timeval *itv, struct timeval *otv)
+__adjtime64 (const struct __timeval64 *itv, struct __timeval64 *otv)
 {
-  struct timex tntx;
+  struct __timex64 tntx;
 
   if (itv)
     {
-      struct timeval tmp;
+      struct __timeval64 tmp;
 
       /* We will do some check here. */
       tmp.tv_sec = itv->tv_sec + itv->tv_usec / 1000000L;
@@ -43,7 +43,7 @@ __adjtime (const struct timeval *itv, struct timeval *otv)
   else
     tntx.modes = ADJ_OFFSET_SS_READ;
 
-  if (__glibc_unlikely (__adjtimex (&tntx) < 0))
+  if (__glibc_unlikely (__clock_adjtime64 (CLOCK_REALTIME, &tntx) < 0))
     return -1;
 
   if (otv)
@@ -62,6 +62,24 @@ __adjtime (const struct timeval *itv, struct timeval *otv)
   return 0;
 }
 
+#if __TIMESIZE != 64
+libc_hidden_def (__adjtime64)
+
+int
+__adjtime (const struct timeval *itv, struct timeval *otv)
+{
+  struct __timeval64 itv64, otv64;
+  int retval;
+
+  itv64 = valid_timeval_to_timeval64 (*itv);
+  retval = __adjtime64 (&itv64, otv != NULL ? &otv64 : NULL);
+  if (otv != NULL)
+    *otv = valid_timeval64_to_timeval (otv64);
+
+  return retval;
+}
+#endif
+
 #ifdef VERSION_adjtime
 weak_alias (__adjtime, __wadjtime);
 default_symbol_version (__wadjtime, adjtime, VERSION_adjtime);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
                   ` (2 preceding siblings ...)
  2020-05-08 14:56 ` [PATCH v2 3/7] y2038: linux: Provide __adjtime64 implementation Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-19 19:10   ` Adhemerval Zanella via Libc-alpha
  2020-05-08 14:56 ` [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64 Lukasz Majewski
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This type is a glibc's "internal" type to get time parameters data from
Linux kernel (NTP daemon interface). It stores time in struct __timeval64
rather than struct timeval, which makes it Y2038-proof.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/unix/sysv/linux/include/sys/timex.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index cf3059f63c..8c536b9a95 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -32,6 +32,7 @@ libc_hidden_proto (__adjtimex)
 #   define __timex64 timex
 #   define __clock_adjtime64 __clock_adjtime
 #   define ___adjtimex64 ___adjtimex
+#   define __ntptimeval64 ntptimeval
 #  else
 
 struct __timex64
@@ -77,6 +78,19 @@ extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
 libc_hidden_proto (__clock_adjtime64);
 extern int ___adjtimex64 (struct __timex64 *tx64);
 libc_hidden_proto (___adjtimex64)
+
+struct __ntptimeval64
+{
+  struct __timeval64 time;	/* current time (ro) */
+  long int maxerror;	/* maximum error (us) (ro) */
+  long int esterror;	/* estimated error (us) (ro) */
+  long int tai;		/* TAI offset (ro) */
+
+  long int __glibc_reserved1;
+  long int __glibc_reserved2;
+  long int __glibc_reserved3;
+  long int __glibc_reserved4;
+};
 #  endif
 
 /* Convert a known valid struct timex into a struct __timex64.  */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
                   ` (3 preceding siblings ...)
  2020-05-08 14:56 ` [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-19 19:12   ` Adhemerval Zanella via Libc-alpha
  2020-05-08 14:56 ` [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation Lukasz Majewski
  2020-05-08 14:56 ` [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation Lukasz Majewski
  6 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

Those functions allow easy conversion between Y2038 safe, glibc internal
struct __ntptimeval64 and struct ntptimeval.

The reserved fields (i.e. __glibc_reserved{1234}) during conversion are
zeroed as well, to provide behavior similar to one in ntp_gettimex function
(where those are cleared before the struct ntptimeval is returned).

Those functions are put in Linux specific sys/timex.h file, as putting
them into glibc's local include/time.h would cause build break on HURD as
it doesn't support struct timex related syscalls.

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/unix/sysv/linux/include/sys/timex.h | 36 +++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index 8c536b9a95..e762e03230 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -152,5 +152,41 @@ valid_timex64_to_timex (const struct __timex64 tx64)
 
   return tx;
 }
+
+/* Convert a known valid struct ntptimeval into a struct __ntptimeval64.  */
+static inline struct __ntptimeval64
+valid_ntptimeval_to_ntptimeval64 (const struct ntptimeval ntv)
+{
+  struct __ntptimeval64 ntv64;
+
+  ntv64.time = valid_timeval_to_timeval64 (ntv.time);
+  ntv64.maxerror = ntv.maxerror;
+  ntv64.esterror = ntv.esterror;
+  ntv64.tai = ntv.tai;
+  ntv64.__glibc_reserved1 = 0;
+  ntv64.__glibc_reserved2 = 0;
+  ntv64.__glibc_reserved3 = 0;
+  ntv64.__glibc_reserved4 = 0;
+
+  return ntv64;
+}
+
+/* Convert a known valid struct __ntptimeval64 into a struct ntptimeval.  */
+static inline struct ntptimeval
+valid_ntptimeval64_to_ntptimeval (const struct __ntptimeval64 ntp64)
+{
+  struct ntptimeval ntp;
+
+  ntp.time = valid_timeval64_to_timeval (ntp64.time);
+  ntp.maxerror = ntp64.maxerror;
+  ntp.esterror = ntp64.esterror;
+  ntp.tai = ntp64.tai;
+  ntp.__glibc_reserved1 = 0;
+  ntp.__glibc_reserved2 = 0;
+  ntp.__glibc_reserved3 = 0;
+  ntp.__glibc_reserved4 = 0;
+
+  return ntp;
+}
 # endif /* _ISOMAC */
 #endif /* sys/timex.h */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
                   ` (4 preceding siblings ...)
  2020-05-08 14:56 ` [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64 Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-19 19:18   ` Adhemerval Zanella via Libc-alpha
  2020-05-08 14:56 ` [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation Lukasz Majewski
  6 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch provides new __ntp_gettime64 explicit 64 bit function for getting
time parameters via NTP interface.

Internally, the __clock_adjtime64 syscall is used instead of __adjtimex. This
patch is necessary for having architectures with __WORDSIZE == 32 Y2038 safe.

Moreover, a 32 bit version - __ntp_gettime has been refactored to internally
use __ntp_gettime64.

The __ntp_gettime is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
ntptimeval and 64 bit struct __ntptimeval64.

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without to
test the proper usage of both __ntp_gettime64 and __ntp_gettime.
---
 sysdeps/unix/sysv/linux/include/sys/timex.h |  4 ++++
 sysdeps/unix/sysv/linux/ntp_gettime.c       | 24 ++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index e762e03230..ef53515803 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -33,6 +33,7 @@ libc_hidden_proto (__adjtimex)
 #   define __clock_adjtime64 __clock_adjtime
 #   define ___adjtimex64 ___adjtimex
 #   define __ntptimeval64 ntptimeval
+#   define __ntp_gettime64 __ntp_gettime
 #  else
 
 struct __timex64
@@ -91,6 +92,9 @@ struct __ntptimeval64
   long int __glibc_reserved3;
   long int __glibc_reserved4;
 };
+extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
+libc_hidden_proto (__ntp_gettime64)
+
 #  endif
 
 /* Convert a known valid struct timex into a struct __timex64.  */
diff --git a/sysdeps/unix/sysv/linux/ntp_gettime.c b/sysdeps/unix/sysv/linux/ntp_gettime.c
index c8d6a197dc..21aeffadeb 100644
--- a/sysdeps/unix/sysv/linux/ntp_gettime.c
+++ b/sysdeps/unix/sysv/linux/ntp_gettime.c
@@ -17,6 +17,7 @@
 
 #define ntp_gettime ntp_gettime_redirect
 
+#include <time.h>
 #include <sys/timex.h>
 
 #undef ntp_gettime
@@ -27,15 +28,32 @@
 
 
 int
-ntp_gettime (struct ntptimeval *ntv)
+__ntp_gettime64 (struct __ntptimeval64 *ntv)
 {
-  struct timex tntx;
+  struct __timex64 tntx;
   int result;
 
   tntx.modes = 0;
-  result = __adjtimex (&tntx);
+  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
   ntv->time = tntx.time;
   ntv->maxerror = tntx.maxerror;
   ntv->esterror = tntx.esterror;
   return result;
 }
+
+#if __TIMESIZE != 64
+libc_hidden_def (__ntp_gettime64)
+
+int
+__ntp_gettime (struct ntptimeval *ntv)
+{
+  struct __ntptimeval64 ntv64;
+  int result;
+
+  result = __ntp_gettime64 (&ntv64);
+  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
+
+  return result;
+}
+#endif
+strong_alias (__ntp_gettime, ntp_gettime)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation
  2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
                   ` (5 preceding siblings ...)
  2020-05-08 14:56 ` [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation Lukasz Majewski
@ 2020-05-08 14:56 ` Lukasz Majewski
  2020-05-19 19:19   ` Adhemerval Zanella via Libc-alpha
  6 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-08 14:56 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis

This patch provides new __ntp_gettimex64 explicit 64 bit function for getting
time parameters via NTP interface.

The call to __adjtimex in __ntp_gettime64 function has been replaced with
direct call to __clock_adjtime64 syscall, to simplify the code.

Moreover, a 32 bit version - __ntp_gettimex has been refactored to internally
use __ntp_gettimex64.

The __ntp_gettimex is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
ntptimeval and 64 bit struct __ntptimeval64.

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without to
test the proper usage of both __ntp_gettimex64 and __ntp_gettimex.
---
 sysdeps/unix/sysv/linux/include/sys/timex.h |  3 +++
 sysdeps/unix/sysv/linux/ntp_gettimex.c      | 24 ++++++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
index ef53515803..335a38c3bd 100644
--- a/sysdeps/unix/sysv/linux/include/sys/timex.h
+++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
@@ -34,6 +34,7 @@ libc_hidden_proto (__adjtimex)
 #   define ___adjtimex64 ___adjtimex
 #   define __ntptimeval64 ntptimeval
 #   define __ntp_gettime64 __ntp_gettime
+#   define __ntp_gettimex64 __ntp_gettimex
 #  else
 
 struct __timex64
@@ -94,6 +95,8 @@ struct __ntptimeval64
 };
 extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
 libc_hidden_proto (__ntp_gettime64)
+extern int __ntp_gettimex64 (struct __ntptimeval64 *ntv);
+libc_hidden_proto (__ntp_gettimex64)
 
 #  endif
 
diff --git a/sysdeps/unix/sysv/linux/ntp_gettimex.c b/sysdeps/unix/sysv/linux/ntp_gettimex.c
index 0f26d4806e..7d0328c6ca 100644
--- a/sysdeps/unix/sysv/linux/ntp_gettimex.c
+++ b/sysdeps/unix/sysv/linux/ntp_gettimex.c
@@ -15,6 +15,7 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <time.h>
 #include <sys/timex.h>
 
 #ifndef MOD_OFFSET
@@ -23,13 +24,13 @@
 
 
 int
-ntp_gettimex (struct ntptimeval *ntv)
+__ntp_gettimex64 (struct __ntptimeval64 *ntv)
 {
-  struct timex tntx;
+  struct __timex64 tntx;
   int result;
 
   tntx.modes = 0;
-  result = __adjtimex (&tntx);
+  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
   ntv->time = tntx.time;
   ntv->maxerror = tntx.maxerror;
   ntv->esterror = tntx.esterror;
@@ -40,3 +41,20 @@ ntp_gettimex (struct ntptimeval *ntv)
   ntv->__glibc_reserved4 = 0;
   return result;
 }
+
+#if __TIMESIZE != 64
+libc_hidden_def (__ntp_gettimex64)
+
+int
+__ntp_gettimex (struct ntptimeval *ntv)
+{
+  struct __ntptimeval64 ntv64;
+  int result;
+
+  result = __ntp_gettimex64 (&ntv64);
+  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
+
+  return result;
+}
+#endif
+strong_alias (__ntp_gettimex, ntp_gettimex)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation
  2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
@ 2020-05-15 10:03   ` Lukasz Majewski
  2020-05-19 19:07   ` Adhemerval Zanella via Libc-alpha
  1 sibling, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-15 10:03 UTC (permalink / raw)
  To: Joseph Myers, Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab

[-- Attachment #1: Type: text/plain, Size: 7798 bytes --]

Hi,

> This patch replaces auto generated wrapper (as described in
> sysdeps/unix/sysv/linux/syscalls.list) for clock_adjtime with one
> which adds extra support for reading 64 bit time values on machines
> with __TIMESIZE != 64.
> 
> To achieve this goal new __clock_adjtime64 explicit 64 bit function
> for adjusting Linux clock has been added.
> Moreover, a 32 bit version - __clock_adjtime has been refactored to
> internally use __clock_adjtime64.
> 
> The __clock_adjtime is now supposed to be used on systems still
> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> conversions between 64 bit struct __timespec64 and struct timespec.
> 
> The new __clock_adjtime64 syscall available from Linux 5.1+ has been
> used, when applicable.
> Up till v5.4 in the Linux kernel there was a bug preventing this call
> from obtaining correct struct's timex time.tv_sec time after time_t
> overflow (i.e. not being Y2038 safe).
> 
> Build tests:
> - ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Linux kernel, headers and minimal kernel version for glibc build test
> matrix:
> - Linux v5.1 (with clock_adjtime64) and glibc build with v5.1 as
>   minimal kernel version (--enable-kernel="5.1.0")
>   The __ASSUME_TIME64_SYSCALLS flag defined.
> 
> - Linux v5.1 and default minimal kernel version
>   The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports
> clock_adjtime64 syscall.
> 
> - Linux v4.19 (no clock_adjtime64 support) with default minimal
> kernel version for contemporary glibc (3.2.0)
>   This kernel doesn't support clock_adjtime64 syscall, so the
> fallback to clock_adjtime is tested.
> 
> Above tests were performed with Y2038 redirection applied as well as
> without (so the __TIMESIZE != 64 execution path is checked as well).
> 
> No regressions were observed.
> 
> Changes for v2:
> - Fix wrong indentation
> - Remove the check for (ret >= TIME_OK && ret <= TIME_ERROR) in the
>   clock_adjtime64 call - just check if syscall returned ENOSYS
> - Add 64 bit time overflow for ADJ_SETOFFSET set in modes

Adhemerval, is this patch OK for pulling? Shall I change anything?

> ---
>  sysdeps/unix/sysv/linux/Makefile            |  2 +-
>  sysdeps/unix/sysv/linux/clock_adjtime.c     | 71
> +++++++++++++++++++++ sysdeps/unix/sysv/linux/include/sys/timex.h |
> 3 + sysdeps/unix/sysv/linux/syscalls.list       |  1 -
>  4 files changed, 75 insertions(+), 2 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile
> b/sysdeps/unix/sysv/linux/Makefile index 0326f92c40..2f2fd5d4d0 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -60,7 +60,7 @@ sysdep_routines += adjtimex clone umount umount2
> readahead sysctl \ personality epoll_wait tee vmsplice splice \
>  		   open_by_handle_at mlock2 pkey_mprotect pkey_set
> pkey_get \ timerfd_gettime timerfd_settime prctl \
> -		   process_vm_readv process_vm_writev
> +		   process_vm_readv process_vm_writev clock_adjtime
>  
>  CFLAGS-gethostid.c = -fexceptions
>  CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables
> diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c
> b/sysdeps/unix/sysv/linux/clock_adjtime.c new file mode 100644
> index 0000000000..5c02031bb5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/clock_adjtime.c
> @@ -0,0 +1,71 @@
> +/* clock_adjtime -- tune kernel clock
> +   Copyright (C) 2020 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; see the file COPYING.LIB.
> If
> +   not, see <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sysdep.h>
> +#include <sys/timex.h>
> +#include <kernel-features.h>
> +
> +int
> +__clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_clock_adjtime64
> +#  define __NR_clock_adjtime64 __NR_clock_adjtime
> +# endif
> +	return INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> +#else
> +  int ret = INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> +  if (errno != ENOSYS)
> +    return ret;
> +
> +  if (tx64->modes & ADJ_SETOFFSET
> +      && ! in_time_t_range (tx64->time.tv_sec))
> +    {
> +      __set_errno (EOVERFLOW);
> +      return -1;
> +    }
> +
> +  struct timex tx32 = valid_timex64_to_timex (*tx64);
> +  int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
> +  *tx64 = valid_timex_to_timex64 (tx32);
> +
> +  return retval;
> +#endif
> +}
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__clock_adjtime64)
> +
> +int
> +__clock_adjtime (const clockid_t clock_id, struct timex *tx)
> +{
> +  struct __timex64 tx64;
> +  int retval;
> +
> +  tx64 = valid_timex_to_timex64 (*tx);
> +  retval = __clock_adjtime64 (clock_id, &tx64);
> +  *tx = valid_timex64_to_timex (tx64);
> +
> +  return retval;
> +}
> +#endif
> +libc_hidden_def (__clock_adjtime);
> +strong_alias (__clock_adjtime, clock_adjtime)
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h
> b/sysdeps/unix/sysv/linux/include/sys/timex.h index
> 3efe7cd306..2848c5cf76 100644 ---
> a/sysdeps/unix/sysv/linux/include/sys/timex.h +++
> b/sysdeps/unix/sysv/linux/include/sys/timex.h @@ -30,6 +30,7 @@
> libc_hidden_proto (__adjtimex) /* Local definition of 64 bit time
> supporting timex struct */ #  if __TIMESIZE == 64
>  #   define __timex64 timex
> +#   define __clock_adjtime64 __clock_adjtime
>  #  else
>  
>  struct __timex64
> @@ -71,6 +72,8 @@ struct __timex64
>    int  :32;
>    int  :32;
>  };
> +extern int __clock_adjtime64 (const clockid_t clock_id, struct
> __timex64 *tx64); +libc_hidden_proto (__clock_adjtime64);
>  #  endif
>  
>  /* Convert a known valid struct timex into a struct __timex64.  */
> diff --git a/sysdeps/unix/sysv/linux/syscalls.list
> b/sysdeps/unix/sysv/linux/syscalls.list index 1d8893d1b8..01ec2bfa95
> 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -4,7 +4,6 @@ alarm		-	alarm
> i:i	alarm bdflush		EXTRA	bdflush
> 	i:ii	__compat_bdflush
> bdflush@GLIBC_2.0:GLIBC_2.23 capget		EXTRA
> capget		i:pp	capget capset
> EXTRA	capset		i:pp	capset
> -clock_adjtime	EXTRA	clock_adjtime	i:ip
> __clock_adjtime		clock_adjtime create_module
> EXTRA	create_module	3
> __compat_create_module	create_module@GLIBC_2.0:GLIBC_2.23
> delete_module	EXTRA	delete_module	3
> delete_module epoll_create	EXTRA	epoll_create
> i:i	epoll_create




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation
  2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
  2020-05-15 10:03   ` Lukasz Majewski
@ 2020-05-19 19:07   ` Adhemerval Zanella via Libc-alpha
  2020-05-19 19:58     ` Lukasz Majewski
  1 sibling, 1 reply; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 19:07 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab



On 08/05/2020 11:56, Lukasz Majewski wrote:
> This patch replaces auto generated wrapper (as described in
> sysdeps/unix/sysv/linux/syscalls.list) for clock_adjtime with one which adds
> extra support for reading 64 bit time values on machines with __TIMESIZE != 64.
> 
> To achieve this goal new __clock_adjtime64 explicit 64 bit function for
> adjusting Linux clock has been added.
> Moreover, a 32 bit version - __clock_adjtime has been refactored to internally
> use __clock_adjtime64.
> 
> The __clock_adjtime is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversions between 64 bit
> struct __timespec64 and struct timespec.
> 
> The new __clock_adjtime64 syscall available from Linux 5.1+ has been used, when
> applicable.
> Up till v5.4 in the Linux kernel there was a bug preventing this call from
> obtaining correct struct's timex time.tv_sec time after time_t overflow
> (i.e. not being Y2038 safe).
> 
> Build tests:
> - ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Linux kernel, headers and minimal kernel version for glibc build test matrix:
> - Linux v5.1 (with clock_adjtime64) and glibc build with v5.1 as
>   minimal kernel version (--enable-kernel="5.1.0")
>   The __ASSUME_TIME64_SYSCALLS flag defined.
> 
> - Linux v5.1 and default minimal kernel version
>   The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports clock_adjtime64
>   syscall.
> 
> - Linux v4.19 (no clock_adjtime64 support) with default minimal kernel version
>   for contemporary glibc (3.2.0)
>   This kernel doesn't support clock_adjtime64 syscall, so the fallback to
>   clock_adjtime is tested.
> 
> Above tests were performed with Y2038 redirection applied as well as without
> (so the __TIMESIZE != 64 execution path is checked as well).
> 
> No regressions were observed.
> 
> Changes for v2:
> - Fix wrong indentation
> - Remove the check for (ret >= TIME_OK && ret <= TIME_ERROR) in the
>   clock_adjtime64 call - just check if syscall returned ENOSYS
> - Add 64 bit time overflow for ADJ_SETOFFSET set in modes

Ok with the changes below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/Makefile            |  2 +-
>  sysdeps/unix/sysv/linux/clock_adjtime.c     | 71 +++++++++++++++++++++
>  sysdeps/unix/sysv/linux/include/sys/timex.h |  3 +
>  sysdeps/unix/sysv/linux/syscalls.list       |  1 -
>  4 files changed, 75 insertions(+), 2 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 0326f92c40..2f2fd5d4d0 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -60,7 +60,7 @@ sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
>  		   personality epoll_wait tee vmsplice splice \
>  		   open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \
>  		   timerfd_gettime timerfd_settime prctl \
> -		   process_vm_readv process_vm_writev
> +		   process_vm_readv process_vm_writev clock_adjtime
>  
>  CFLAGS-gethostid.c = -fexceptions
>  CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables

Ok.

> diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c b/sysdeps/unix/sysv/linux/clock_adjtime.c
> new file mode 100644
> index 0000000000..5c02031bb5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/clock_adjtime.c
> @@ -0,0 +1,71 @@
> +/* clock_adjtime -- tune kernel clock

I think we usually add a final period in one line description.

> +   Copyright (C) 2020 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; see the file COPYING.LIB.  If
> +   not, see <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sysdep.h>
> +#include <sys/timex.h>
> +#include <kernel-features.h>
> +
> +int
> +__clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_clock_adjtime64
> +#  define __NR_clock_adjtime64 __NR_clock_adjtime
> +# endif
> +	return INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> +#else
> +  int ret = INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> +  if (errno != ENOSYS)
> +    return ret;
> +
> +  if (tx64->modes & ADJ_SETOFFSET
> +      && ! in_time_t_range (tx64->time.tv_sec))
> +    {
> +      __set_errno (EOVERFLOW);
> +      return -1;
> +    }
> +
> +  struct timex tx32 = valid_timex64_to_timex (*tx64);
> +  int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
> +  *tx64 = valid_timex_to_timex64 (tx32);> +
> +  return retval;

The function might still return EINVAL for invalid clocks, ENODEV for
invalid dynamic ones, EOPNOTSUPP if the clock does not support adjustment,
or EPERM if buf.modes is neither 0 nor ADJ_OFFSET_SS_READ and the caller 
does not have sufficient privilege.

So we still need to check the syscall return value prior setting the
tx64 value:

  int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
  if (retval >= 0)
    *tx64 = valid_timex_to_timex64 (tx32);
  return retval;

Ok with this change.

> +#endif
> +}
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__clock_adjtime64)
> +
> +int
> +__clock_adjtime (const clockid_t clock_id, struct timex *tx)
> +{
> +  struct __timex64 tx64;
> +  int retval;
> +
> +  tx64 = valid_timex_to_timex64 (*tx);
> +  retval = __clock_adjtime64 (clock_id, &tx64);

Same as before, we need to check its return value prior setting TX:

  retval = __clock_adjtime64 (clock_id, &tx64);
  if (retval >= 0)
     *tx = valid_timex64_to_timex (tx64);
  return retval;

Ok with this change.
  
> +  *tx = valid_timex64_to_timex (tx64);
> +
> +  return retval;
> +}
> +#endif
> +libc_hidden_def (__clock_adjtime);
> +strong_alias (__clock_adjtime, clock_adjtime)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index 3efe7cd306..2848c5cf76 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -30,6 +30,7 @@ libc_hidden_proto (__adjtimex)
>  /* Local definition of 64 bit time supporting timex struct */
>  #  if __TIMESIZE == 64
>  #   define __timex64 timex
> +#   define __clock_adjtime64 __clock_adjtime
>  #  else
>  
>  struct __timex64
> @@ -71,6 +72,8 @@ struct __timex64
>    int  :32;
>    int  :32;
>  };
> +extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
> +libc_hidden_proto (__clock_adjtime64);
>  #  endif
>  
>  /* Convert a known valid struct timex into a struct __timex64.  */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index 1d8893d1b8..01ec2bfa95 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -4,7 +4,6 @@ alarm		-	alarm		i:i	alarm
>  bdflush		EXTRA	bdflush		i:ii	__compat_bdflush	bdflush@GLIBC_2.0:GLIBC_2.23
>  capget		EXTRA	capget		i:pp	capget
>  capset		EXTRA	capset		i:pp	capset
> -clock_adjtime	EXTRA	clock_adjtime	i:ip	__clock_adjtime		clock_adjtime
>  create_module	EXTRA	create_module	3	__compat_create_module	create_module@GLIBC_2.0:GLIBC_2.23
>  delete_module	EXTRA	delete_module	3	delete_module
>  epoll_create	EXTRA	epoll_create	i:i	epoll_create
> 

Ok.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type
  2020-05-08 14:56 ` [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type Lukasz Majewski
@ 2020-05-19 19:10   ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 19:10 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab



On 08/05/2020 11:56, Lukasz Majewski wrote:
> This type is a glibc's "internal" type to get time parameters data from
> Linux kernel (NTP daemon interface). It stores time in struct __timeval64
> rather than struct timeval, which makes it Y2038-proof.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/include/sys/timex.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index cf3059f63c..8c536b9a95 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -32,6 +32,7 @@ libc_hidden_proto (__adjtimex)
>  #   define __timex64 timex
>  #   define __clock_adjtime64 __clock_adjtime
>  #   define ___adjtimex64 ___adjtimex
> +#   define __ntptimeval64 ntptimeval
>  #  else
>  
>  struct __timex64
> @@ -77,6 +78,19 @@ extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
>  libc_hidden_proto (__clock_adjtime64);
>  extern int ___adjtimex64 (struct __timex64 *tx64);
>  libc_hidden_proto (___adjtimex64)
> +
> +struct __ntptimeval64
> +{
> +  struct __timeval64 time;	/* current time (ro) */
> +  long int maxerror;	/* maximum error (us) (ro) */
> +  long int esterror;	/* estimated error (us) (ro) */
> +  long int tai;		/* TAI offset (ro) */
> +
> +  long int __glibc_reserved1;
> +  long int __glibc_reserved2;
> +  long int __glibc_reserved3;
> +  long int __glibc_reserved4;
> +};
>  #  endif
>  
>  /* Convert a known valid struct timex into a struct __timex64.  */
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64
  2020-05-08 14:56 ` [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64 Lukasz Majewski
@ 2020-05-19 19:12   ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 19:12 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab



On 08/05/2020 11:56, Lukasz Majewski wrote:
> Those functions allow easy conversion between Y2038 safe, glibc internal
> struct __ntptimeval64 and struct ntptimeval.
> 
> The reserved fields (i.e. __glibc_reserved{1234}) during conversion are
> zeroed as well, to provide behavior similar to one in ntp_gettimex function
> (where those are cleared before the struct ntptimeval is returned).
> 
> Those functions are put in Linux specific sys/timex.h file, as putting
> them into glibc's local include/time.h would cause build break on HURD as
> it doesn't support struct timex related syscalls.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/include/sys/timex.h | 36 +++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index 8c536b9a95..e762e03230 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -152,5 +152,41 @@ valid_timex64_to_timex (const struct __timex64 tx64)
>  
>    return tx;
>  }
> +
> +/* Convert a known valid struct ntptimeval into a struct __ntptimeval64.  */
> +static inline struct __ntptimeval64
> +valid_ntptimeval_to_ntptimeval64 (const struct ntptimeval ntv)
> +{
> +  struct __ntptimeval64 ntv64;
> +
> +  ntv64.time = valid_timeval_to_timeval64 (ntv.time);
> +  ntv64.maxerror = ntv.maxerror;
> +  ntv64.esterror = ntv.esterror;
> +  ntv64.tai = ntv.tai;
> +  ntv64.__glibc_reserved1 = 0;
> +  ntv64.__glibc_reserved2 = 0;
> +  ntv64.__glibc_reserved3 = 0;
> +  ntv64.__glibc_reserved4 = 0;
> +
> +  return ntv64;
> +}
> +

OK.

> +/* Convert a known valid struct __ntptimeval64 into a struct ntptimeval.  */
> +static inline struct ntptimeval
> +valid_ntptimeval64_to_ntptimeval (const struct __ntptimeval64 ntp64)
> +{
> +  struct ntptimeval ntp;
> +
> +  ntp.time = valid_timeval64_to_timeval (ntp64.time);
> +  ntp.maxerror = ntp64.maxerror;
> +  ntp.esterror = ntp64.esterror;
> +  ntp.tai = ntp64.tai;
> +  ntp.__glibc_reserved1 = 0;
> +  ntp.__glibc_reserved2 = 0;
> +  ntp.__glibc_reserved3 = 0;
> +  ntp.__glibc_reserved4 = 0;
> +
> +  return ntp;
> +}
>  # endif /* _ISOMAC */
>  #endif /* sys/timex.h */
> 

Ok.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-08 14:56 ` [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation Lukasz Majewski
@ 2020-05-19 19:18   ` Adhemerval Zanella via Libc-alpha
  2020-05-19 20:20     ` Lukasz Majewski
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 19:18 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab



On 08/05/2020 11:56, Lukasz Majewski wrote:
> This patch provides new __ntp_gettime64 explicit 64 bit function for getting
> time parameters via NTP interface.
> 
> Internally, the __clock_adjtime64 syscall is used instead of __adjtimex. This
> patch is necessary for having architectures with __WORDSIZE == 32 Y2038 safe.
> 
> Moreover, a 32 bit version - __ntp_gettime has been refactored to internally
> use __ntp_gettime64.
> 
> The __ntp_gettime is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
> ntptimeval and 64 bit struct __ntptimeval64.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without to
> test the proper usage of both __ntp_gettime64 and __ntp_gettime.

Ok with a doubt below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/include/sys/timex.h |  4 ++++
>  sysdeps/unix/sysv/linux/ntp_gettime.c       | 24 ++++++++++++++++++---
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index e762e03230..ef53515803 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -33,6 +33,7 @@ libc_hidden_proto (__adjtimex)
>  #   define __clock_adjtime64 __clock_adjtime
>  #   define ___adjtimex64 ___adjtimex
>  #   define __ntptimeval64 ntptimeval
> +#   define __ntp_gettime64 __ntp_gettime
>  #  else
>  
>  struct __timex64
> @@ -91,6 +92,9 @@ struct __ntptimeval64
>    long int __glibc_reserved3;
>    long int __glibc_reserved4;
>  };
> +extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
> +libc_hidden_proto (__ntp_gettime64)
> +
>  #  endif
>  
>  /* Convert a known valid struct timex into a struct __timex64.  */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/ntp_gettime.c b/sysdeps/unix/sysv/linux/ntp_gettime.c
> index c8d6a197dc..21aeffadeb 100644
> --- a/sysdeps/unix/sysv/linux/ntp_gettime.c
> +++ b/sysdeps/unix/sysv/linux/ntp_gettime.c
> @@ -17,6 +17,7 @@
>  
>  #define ntp_gettime ntp_gettime_redirect
>  
> +#include <time.h>
>  #include <sys/timex.h>
>  
>  #undef ntp_gettime
> @@ -27,15 +28,32 @@
>  
>  
>  int
> -ntp_gettime (struct ntptimeval *ntv)
> +__ntp_gettime64 (struct __ntptimeval64 *ntv)
>  {
> -  struct timex tntx;
> +  struct __timex64 tntx;
>    int result;
>  
>    tntx.modes = 0;
> -  result = __adjtimex (&tntx);
> +  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
>    ntv->time = tntx.time;
>    ntv->maxerror = tntx.maxerror;
>    ntv->esterror = tntx.esterror;
>    return result;
>  }

Ok. Maybe add a comment stating that using CLOCK_REALTIME should
not make the function fail with EINVAL, ENODEV, or EOPNOTSUPP.
I am not sure about EPERM in this situation, should we check for
that and avoid seeting NTV in such situation?

> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__ntp_gettime64)
> +
> +int
> +__ntp_gettime (struct ntptimeval *ntv)
> +{
> +  struct __ntptimeval64 ntv64;
> +  int result;
> +
> +  result = __ntp_gettime64 (&ntv64);
> +  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
> +
> +  return result;
> +}
> +#endif
> +strong_alias (__ntp_gettime, ntp_gettime)
> 

Ok.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation
  2020-05-08 14:56 ` [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation Lukasz Majewski
@ 2020-05-19 19:19   ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 19:19 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers
  Cc: Florian Weimer, GNU C Library, Alistair Francis, Andreas Schwab



On 08/05/2020 11:56, Lukasz Majewski wrote:
> This patch provides new __ntp_gettimex64 explicit 64 bit function for getting
> time parameters via NTP interface.
> 
> The call to __adjtimex in __ntp_gettime64 function has been replaced with
> direct call to __clock_adjtime64 syscall, to simplify the code.
> 
> Moreover, a 32 bit version - __ntp_gettimex has been refactored to internally
> use __ntp_gettimex64.
> 
> The __ntp_gettimex is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversions between struct
> ntptimeval and 64 bit struct __ntptimeval64.
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without to
> test the proper usage of both __ntp_gettimex64 and __ntp_gettimex.

Ok with a doubt below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/include/sys/timex.h |  3 +++
>  sysdeps/unix/sysv/linux/ntp_gettimex.c      | 24 ++++++++++++++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index ef53515803..335a38c3bd 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -34,6 +34,7 @@ libc_hidden_proto (__adjtimex)
>  #   define ___adjtimex64 ___adjtimex
>  #   define __ntptimeval64 ntptimeval
>  #   define __ntp_gettime64 __ntp_gettime
> +#   define __ntp_gettimex64 __ntp_gettimex
>  #  else
>  
>  struct __timex64
> @@ -94,6 +95,8 @@ struct __ntptimeval64
>  };
>  extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
>  libc_hidden_proto (__ntp_gettime64)
> +extern int __ntp_gettimex64 (struct __ntptimeval64 *ntv);
> +libc_hidden_proto (__ntp_gettimex64)
>  
>  #  endif
>  
> diff --git a/sysdeps/unix/sysv/linux/ntp_gettimex.c b/sysdeps/unix/sysv/linux/ntp_gettimex.c
> index 0f26d4806e..7d0328c6ca 100644
> --- a/sysdeps/unix/sysv/linux/ntp_gettimex.c
> +++ b/sysdeps/unix/sysv/linux/ntp_gettimex.c
> @@ -15,6 +15,7 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <time.h>
>  #include <sys/timex.h>
>  
>  #ifndef MOD_OFFSET
> @@ -23,13 +24,13 @@
>  
>  
>  int
> -ntp_gettimex (struct ntptimeval *ntv)
> +__ntp_gettimex64 (struct __ntptimeval64 *ntv)
>  {
> -  struct timex tntx;
> +  struct __timex64 tntx;
>    int result;
>  
>    tntx.modes = 0;
> -  result = __adjtimex (&tntx);
> +  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
>    ntv->time = tntx.time;
>    ntv->maxerror = tntx.maxerror;
>    ntv->esterror = tntx.esterror;
> @@ -40,3 +41,20 @@ ntp_gettimex (struct ntptimeval *ntv)
>    ntv->__glibc_reserved4 = 0;
>    return result;
>  }

Ok. As for ntp_gettimex64, maybe add a comment stating that using 
CLOCK_REALTIME should not make the function fail with EINVAL, ENODEV, 
or EOPNOTSUPP.  I am not sure about EPERM in this situation, should 
we check for that and avoid seeting NTV in such situation?

> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__ntp_gettimex64)
> +
> +int
> +__ntp_gettimex (struct ntptimeval *ntv)
> +{
> +  struct __ntptimeval64 ntv64;
> +  int result;
> +
> +  result = __ntp_gettimex64 (&ntv64);
> +  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
> +
> +  return result;
> +}
> +#endif
> +strong_alias (__ntp_gettimex, ntp_gettimex)
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation
  2020-05-19 19:07   ` Adhemerval Zanella via Libc-alpha
@ 2020-05-19 19:58     ` Lukasz Majewski
  0 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-19 19:58 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis,
	Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 9300 bytes --]

Hi Adhemerval,

> On 08/05/2020 11:56, Lukasz Majewski wrote:
> > This patch replaces auto generated wrapper (as described in
> > sysdeps/unix/sysv/linux/syscalls.list) for clock_adjtime with one
> > which adds extra support for reading 64 bit time values on machines
> > with __TIMESIZE != 64.
> > 
> > To achieve this goal new __clock_adjtime64 explicit 64 bit function
> > for adjusting Linux clock has been added.
> > Moreover, a 32 bit version - __clock_adjtime has been refactored to
> > internally use __clock_adjtime64.
> > 
> > The __clock_adjtime is now supposed to be used on systems still
> > supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> > conversions between 64 bit struct __timespec64 and struct timespec.
> > 
> > The new __clock_adjtime64 syscall available from Linux 5.1+ has
> > been used, when applicable.
> > Up till v5.4 in the Linux kernel there was a bug preventing this
> > call from obtaining correct struct's timex time.tv_sec time after
> > time_t overflow (i.e. not being Y2038 safe).
> > 
> > Build tests:
> > - ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> >   https://github.com/lmajewski/meta-y2038 and run tests:
> >   https://github.com/lmajewski/y2038-tests/commits/master
> > 
> > Linux kernel, headers and minimal kernel version for glibc build
> > test matrix:
> > - Linux v5.1 (with clock_adjtime64) and glibc build with v5.1 as
> >   minimal kernel version (--enable-kernel="5.1.0")
> >   The __ASSUME_TIME64_SYSCALLS flag defined.
> > 
> > - Linux v5.1 and default minimal kernel version
> >   The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports
> > clock_adjtime64 syscall.
> > 
> > - Linux v4.19 (no clock_adjtime64 support) with default minimal
> > kernel version for contemporary glibc (3.2.0)
> >   This kernel doesn't support clock_adjtime64 syscall, so the
> > fallback to clock_adjtime is tested.
> > 
> > Above tests were performed with Y2038 redirection applied as well
> > as without (so the __TIMESIZE != 64 execution path is checked as
> > well).
> > 
> > No regressions were observed.
> > 
> > Changes for v2:
> > - Fix wrong indentation
> > - Remove the check for (ret >= TIME_OK && ret <= TIME_ERROR) in the
> >   clock_adjtime64 call - just check if syscall returned ENOSYS
> > - Add 64 bit time overflow for ADJ_SETOFFSET set in modes  
> 
> Ok with the changes below.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
> > ---
> >  sysdeps/unix/sysv/linux/Makefile            |  2 +-
> >  sysdeps/unix/sysv/linux/clock_adjtime.c     | 71
> > +++++++++++++++++++++ sysdeps/unix/sysv/linux/include/sys/timex.h |
> >  3 + sysdeps/unix/sysv/linux/syscalls.list       |  1 -
> >  4 files changed, 75 insertions(+), 2 deletions(-)
> >  create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c
> > 
> > diff --git a/sysdeps/unix/sysv/linux/Makefile
> > b/sysdeps/unix/sysv/linux/Makefile index 0326f92c40..2f2fd5d4d0
> > 100644 --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -60,7 +60,7 @@ sysdep_routines += adjtimex clone umount umount2
> > readahead sysctl \ personality epoll_wait tee vmsplice splice \
> >  		   open_by_handle_at mlock2 pkey_mprotect pkey_set
> > pkey_get \ timerfd_gettime timerfd_settime prctl \
> > -		   process_vm_readv process_vm_writev
> > +		   process_vm_readv process_vm_writev clock_adjtime
> >  
> >  CFLAGS-gethostid.c = -fexceptions
> >  CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c
> > b/sysdeps/unix/sysv/linux/clock_adjtime.c new file mode 100644
> > index 0000000000..5c02031bb5
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/clock_adjtime.c
> > @@ -0,0 +1,71 @@
> > +/* clock_adjtime -- tune kernel clock  
> 
> I think we usually add a final period in one line description.

Ok.

> 
> > +   Copyright (C) 2020 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; see the file COPYING.LIB.
> >  If
> > +   not, see <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <time.h>
> > +#include <sysdep.h>
> > +#include <sys/timex.h>
> > +#include <kernel-features.h>
> > +
> > +int
> > +__clock_adjtime64 (const clockid_t clock_id, struct __timex64
> > *tx64) +{
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +# ifndef __NR_clock_adjtime64
> > +#  define __NR_clock_adjtime64 __NR_clock_adjtime
> > +# endif
> > +	return INLINE_SYSCALL_CALL (clock_adjtime64, clock_id,
> > tx64); +#else
> > +  int ret = INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> > +  if (errno != ENOSYS)
> > +    return ret;
> > +
> > +  if (tx64->modes & ADJ_SETOFFSET
> > +      && ! in_time_t_range (tx64->time.tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }
> > +
> > +  struct timex tx32 = valid_timex64_to_timex (*tx64);
> > +  int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id,
> > &tx32);
> > +  *tx64 = valid_timex_to_timex64 (tx32);> +
> > +  return retval;  
> 
> The function might still return EINVAL for invalid clocks, ENODEV for
> invalid dynamic ones, EOPNOTSUPP if the clock does not support
> adjustment, or EPERM if buf.modes is neither 0 nor ADJ_OFFSET_SS_READ
> and the caller does not have sufficient privilege.
> 
> So we still need to check the syscall return value prior setting the
> tx64 value:
> 
>   int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
>   if (retval >= 0)
>     *tx64 = valid_timex_to_timex64 (tx32);
>   return retval;
> 

Thanks for pointing this out.

> Ok with this change.
> 
> > +#endif
> > +}
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__clock_adjtime64)
> > +
> > +int
> > +__clock_adjtime (const clockid_t clock_id, struct timex *tx)
> > +{
> > +  struct __timex64 tx64;
> > +  int retval;
> > +
> > +  tx64 = valid_timex_to_timex64 (*tx);
> > +  retval = __clock_adjtime64 (clock_id, &tx64);  
> 
> Same as before, we need to check its return value prior setting TX:
> 
>   retval = __clock_adjtime64 (clock_id, &tx64);
>   if (retval >= 0)
>      *tx = valid_timex64_to_timex (tx64);
>   return retval;
> 
> Ok with this change.
>   
> > +  *tx = valid_timex64_to_timex (tx64);
> > +
> > +  return retval;
> > +}
> > +#endif
> > +libc_hidden_def (__clock_adjtime);
> > +strong_alias (__clock_adjtime, clock_adjtime)  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h
> > b/sysdeps/unix/sysv/linux/include/sys/timex.h index
> > 3efe7cd306..2848c5cf76 100644 ---
> > a/sysdeps/unix/sysv/linux/include/sys/timex.h +++
> > b/sysdeps/unix/sysv/linux/include/sys/timex.h @@ -30,6 +30,7 @@
> > libc_hidden_proto (__adjtimex) /* Local definition of 64 bit time
> > supporting timex struct */ #  if __TIMESIZE == 64
> >  #   define __timex64 timex
> > +#   define __clock_adjtime64 __clock_adjtime
> >  #  else
> >  
> >  struct __timex64
> > @@ -71,6 +72,8 @@ struct __timex64
> >    int  :32;
> >    int  :32;
> >  };
> > +extern int __clock_adjtime64 (const clockid_t clock_id, struct
> > __timex64 *tx64); +libc_hidden_proto (__clock_adjtime64);
> >  #  endif
> >  
> >  /* Convert a known valid struct timex into a struct __timex64.  */
> >  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/syscalls.list
> > b/sysdeps/unix/sysv/linux/syscalls.list index
> > 1d8893d1b8..01ec2bfa95 100644 ---
> > a/sysdeps/unix/sysv/linux/syscalls.list +++
> > b/sysdeps/unix/sysv/linux/syscalls.list @@ -4,7 +4,6 @@
> > alarm		-	alarm		i:i	alarm
> > bdflush		EXTRA	bdflush
> > i:ii	__compat_bdflush	bdflush@GLIBC_2.0:GLIBC_2.23
> > capget		EXTRA	capget
> > i:pp	capget capset		EXTRA	capset
> > 	i:pp	capset -clock_adjtime	EXTRA
> > clock_adjtime	i:ip	__clock_adjtime
> > clock_adjtime create_module	EXTRA
> > create_module	3	__compat_create_module
> > create_module@GLIBC_2.0:GLIBC_2.23 delete_module
> > EXTRA	delete_module	3	delete_module
> > epoll_create	EXTRA	epoll_create	i:i
> > epoll_create 
> 
> Ok.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-19 19:18   ` Adhemerval Zanella via Libc-alpha
@ 2020-05-19 20:20     ` Lukasz Majewski
  2020-05-19 20:25       ` Adhemerval Zanella via Libc-alpha
  2020-05-20 15:23       ` Joseph Myers
  0 siblings, 2 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-19 20:20 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis,
	Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 4777 bytes --]

Hi Adhemerval,

> On 08/05/2020 11:56, Lukasz Majewski wrote:
> > This patch provides new __ntp_gettime64 explicit 64 bit function
> > for getting time parameters via NTP interface.
> > 
> > Internally, the __clock_adjtime64 syscall is used instead of
> > __adjtimex. This patch is necessary for having architectures with
> > __WORDSIZE == 32 Y2038 safe.
> > 
> > Moreover, a 32 bit version - __ntp_gettime has been refactored to
> > internally use __ntp_gettime64.
> > 
> > The __ntp_gettime is now supposed to be used on systems still
> > supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> > conversions between struct ntptimeval and 64 bit struct
> > __ntptimeval64.
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> >   https://github.com/lmajewski/meta-y2038 and run tests:
> >   https://github.com/lmajewski/y2038-tests/commits/master
> > 
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test the proper usage of both __ntp_gettime64 and
> > __ntp_gettime.  
> 
> Ok with a doubt below.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
> > ---
> >  sysdeps/unix/sysv/linux/include/sys/timex.h |  4 ++++
> >  sysdeps/unix/sysv/linux/ntp_gettime.c       | 24
> > ++++++++++++++++++--- 2 files changed, 25 insertions(+), 3
> > deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h
> > b/sysdeps/unix/sysv/linux/include/sys/timex.h index
> > e762e03230..ef53515803 100644 ---
> > a/sysdeps/unix/sysv/linux/include/sys/timex.h +++
> > b/sysdeps/unix/sysv/linux/include/sys/timex.h @@ -33,6 +33,7 @@
> > libc_hidden_proto (__adjtimex) #   define __clock_adjtime64
> > __clock_adjtime #   define ___adjtimex64 ___adjtimex
> >  #   define __ntptimeval64 ntptimeval
> > +#   define __ntp_gettime64 __ntp_gettime
> >  #  else
> >  
> >  struct __timex64
> > @@ -91,6 +92,9 @@ struct __ntptimeval64
> >    long int __glibc_reserved3;
> >    long int __glibc_reserved4;
> >  };
> > +extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
> > +libc_hidden_proto (__ntp_gettime64)
> > +
> >  #  endif
> >  
> >  /* Convert a known valid struct timex into a struct __timex64.  */
> >  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/ntp_gettime.c
> > b/sysdeps/unix/sysv/linux/ntp_gettime.c index
> > c8d6a197dc..21aeffadeb 100644 ---
> > a/sysdeps/unix/sysv/linux/ntp_gettime.c +++
> > b/sysdeps/unix/sysv/linux/ntp_gettime.c @@ -17,6 +17,7 @@
> >  
> >  #define ntp_gettime ntp_gettime_redirect
> >  
> > +#include <time.h>
> >  #include <sys/timex.h>
> >  
> >  #undef ntp_gettime
> > @@ -27,15 +28,32 @@
> >  
> >  
> >  int
> > -ntp_gettime (struct ntptimeval *ntv)
> > +__ntp_gettime64 (struct __ntptimeval64 *ntv)
> >  {
> > -  struct timex tntx;
> > +  struct __timex64 tntx;
> >    int result;
> >  
> >    tntx.modes = 0;
> > -  result = __adjtimex (&tntx);
> > +  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
> >    ntv->time = tntx.time;
> >    ntv->maxerror = tntx.maxerror;
> >    ntv->esterror = tntx.esterror;
> >    return result;
> >  }  
> 
> Ok. Maybe add a comment stating that using CLOCK_REALTIME should
> not make the function fail with EINVAL, ENODEV, or EOPNOTSUPP.
> I am not sure about EPERM in this situation, should we check for
> that and avoid seeting NTV in such situation?
> 

I will add following comment:

/* Using the CLOCK_REALTIME with __clock_adjtime64 (as a replacement
for Y2038 unsafe adjtimex) will not make the function fail with EINVAL,
ENODEV, or EOPNOTSUPP.  */

Regarding the EPERM:

The adjtimex also could return EPERM:
http://man7.org/linux/man-pages/man2/adjtimex.2.html

which would be propagated to caller of ntp_gettime. In this case the
ntv structure would get updated.

If we want to preserve the same behavior, it would be correct to leave
the code as is (and ntv would get updated anyway).

> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__ntp_gettime64)
> > +
> > +int
> > +__ntp_gettime (struct ntptimeval *ntv)
> > +{
> > +  struct __ntptimeval64 ntv64;
> > +  int result;
> > +
> > +  result = __ntp_gettime64 (&ntv64);
> > +  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
> > +
> > +  return result;
> > +}
> > +#endif
> > +strong_alias (__ntp_gettime, ntp_gettime)
> >   
> 
> Ok.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-19 20:20     ` Lukasz Majewski
@ 2020-05-19 20:25       ` Adhemerval Zanella via Libc-alpha
  2020-05-20 15:23       ` Joseph Myers
  1 sibling, 0 replies; 21+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-19 20:25 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Florian Weimer, GNU C Library, Andreas Schwab, Alistair Francis,
	Joseph Myers


[-- Attachment #1.1: Type: text/plain, Size: 5094 bytes --]



On 19/05/2020 17:20, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 08/05/2020 11:56, Lukasz Majewski wrote:
>>> This patch provides new __ntp_gettime64 explicit 64 bit function
>>> for getting time parameters via NTP interface.
>>>
>>> Internally, the __clock_adjtime64 syscall is used instead of
>>> __adjtimex. This patch is necessary for having architectures with
>>> __WORDSIZE == 32 Y2038 safe.
>>>
>>> Moreover, a 32 bit version - __ntp_gettime has been refactored to
>>> internally use __ntp_gettime64.
>>>
>>> The __ntp_gettime is now supposed to be used on systems still
>>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
>>> conversions between struct ntptimeval and 64 bit struct
>>> __ntptimeval64.
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>>
>>> Run-time tests:
>>> - Run specific tests on ARM/x86 32bit systems (qemu):
>>>   https://github.com/lmajewski/meta-y2038 and run tests:
>>>   https://github.com/lmajewski/y2038-tests/commits/master
>>>
>>> Above tests were performed with Y2038 redirection applied as well
>>> as without to test the proper usage of both __ntp_gettime64 and
>>> __ntp_gettime.  
>>
>> Ok with a doubt below.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>>
>>> ---
>>>  sysdeps/unix/sysv/linux/include/sys/timex.h |  4 ++++
>>>  sysdeps/unix/sysv/linux/ntp_gettime.c       | 24
>>> ++++++++++++++++++--- 2 files changed, 25 insertions(+), 3
>>> deletions(-)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h
>>> b/sysdeps/unix/sysv/linux/include/sys/timex.h index
>>> e762e03230..ef53515803 100644 ---
>>> a/sysdeps/unix/sysv/linux/include/sys/timex.h +++
>>> b/sysdeps/unix/sysv/linux/include/sys/timex.h @@ -33,6 +33,7 @@
>>> libc_hidden_proto (__adjtimex) #   define __clock_adjtime64
>>> __clock_adjtime #   define ___adjtimex64 ___adjtimex
>>>  #   define __ntptimeval64 ntptimeval
>>> +#   define __ntp_gettime64 __ntp_gettime
>>>  #  else
>>>  
>>>  struct __timex64
>>> @@ -91,6 +92,9 @@ struct __ntptimeval64
>>>    long int __glibc_reserved3;
>>>    long int __glibc_reserved4;
>>>  };
>>> +extern int __ntp_gettime64 (struct __ntptimeval64 *ntv);
>>> +libc_hidden_proto (__ntp_gettime64)
>>> +
>>>  #  endif
>>>  
>>>  /* Convert a known valid struct timex into a struct __timex64.  */
>>>  
>>
>> Ok.
>>
>>> diff --git a/sysdeps/unix/sysv/linux/ntp_gettime.c
>>> b/sysdeps/unix/sysv/linux/ntp_gettime.c index
>>> c8d6a197dc..21aeffadeb 100644 ---
>>> a/sysdeps/unix/sysv/linux/ntp_gettime.c +++
>>> b/sysdeps/unix/sysv/linux/ntp_gettime.c @@ -17,6 +17,7 @@
>>>  
>>>  #define ntp_gettime ntp_gettime_redirect
>>>  
>>> +#include <time.h>
>>>  #include <sys/timex.h>
>>>  
>>>  #undef ntp_gettime
>>> @@ -27,15 +28,32 @@
>>>  
>>>  
>>>  int
>>> -ntp_gettime (struct ntptimeval *ntv)
>>> +__ntp_gettime64 (struct __ntptimeval64 *ntv)
>>>  {
>>> -  struct timex tntx;
>>> +  struct __timex64 tntx;
>>>    int result;
>>>  
>>>    tntx.modes = 0;
>>> -  result = __adjtimex (&tntx);
>>> +  result = __clock_adjtime64 (CLOCK_REALTIME, &tntx);
>>>    ntv->time = tntx.time;
>>>    ntv->maxerror = tntx.maxerror;
>>>    ntv->esterror = tntx.esterror;
>>>    return result;
>>>  }  
>>
>> Ok. Maybe add a comment stating that using CLOCK_REALTIME should
>> not make the function fail with EINVAL, ENODEV, or EOPNOTSUPP.
>> I am not sure about EPERM in this situation, should we check for
>> that and avoid seeting NTV in such situation?
>>
> 
> I will add following comment:
> 
> /* Using the CLOCK_REALTIME with __clock_adjtime64 (as a replacement
> for Y2038 unsafe adjtimex) will not make the function fail with EINVAL,
> ENODEV, or EOPNOTSUPP.  */

Maybe:

  /* clock_adjtime64 with CLOCK_REALTIME does not trigger EINVAL,
     ENODEV, or EOPNOTSUPP.  It might still trigger EPERM.  */

> 
> Regarding the EPERM:
> 
> The adjtimex also could return EPERM:
> http://man7.org/linux/man-pages/man2/adjtimex.2.html
> 
> which would be propagated to caller of ntp_gettime. In this case the
> ntv structure would get updated.
> 
> If we want to preserve the same behavior, it would be correct to leave
> the code as is (and ntv would get updated anyway).

Alight, we can track this in another patch.

> 
>>> +
>>> +#if __TIMESIZE != 64
>>> +libc_hidden_def (__ntp_gettime64)
>>> +
>>> +int
>>> +__ntp_gettime (struct ntptimeval *ntv)
>>> +{
>>> +  struct __ntptimeval64 ntv64;
>>> +  int result;
>>> +
>>> +  result = __ntp_gettime64 (&ntv64);
>>> +  *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
>>> +
>>> +  return result;
>>> +}
>>> +#endif
>>> +strong_alias (__ntp_gettime, ntp_gettime)
>>>   
>>
>> Ok.
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-19 20:20     ` Lukasz Majewski
  2020-05-19 20:25       ` Adhemerval Zanella via Libc-alpha
@ 2020-05-20 15:23       ` Joseph Myers
  2020-05-20 17:08         ` Lukasz Majewski
  1 sibling, 1 reply; 21+ messages in thread
From: Joseph Myers @ 2020-05-20 15:23 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Florian Weimer, Andreas Schwab, Alistair Francis, GNU C Library

I'm seeing build failures from build-many-glibcs.py for hppa, mips (n32), 
sh4, sparcv8, sparcv9, that appear to arise from one of these patches.

../sysdeps/unix/sysv/linux/ntp_gettime.c: In function '__ntp_gettime':
../sysdeps/unix/sysv/linux/ntp_gettime.c:56:10: error: 'ntv64.tai' is used uninitialized in this function [-Werror=uninitialized]
   56 |   *ntv = valid_ntptimeval64_to_ntptimeval (ntv64);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

https://sourceware.org/pipermail/libc-testresults/2020q2/006191.html

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-20 15:23       ` Joseph Myers
@ 2020-05-20 17:08         ` Lukasz Majewski
  2020-05-20 17:21           ` Joseph Myers
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-20 17:08 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Florian Weimer, Andreas Schwab, Alistair Francis, GNU C Library

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

Hi Joseph,

> I'm seeing build failures from build-many-glibcs.py for hppa, mips
> (n32), sh4, sparcv8, sparcv9, that appear to arise from one of these
> patches.
> 
> ../sysdeps/unix/sysv/linux/ntp_gettime.c: In function '__ntp_gettime':
> ../sysdeps/unix/sysv/linux/ntp_gettime.c:56:10: error: 'ntv64.tai' is
> used uninitialized in this function [-Werror=uninitialized] 56 |
> *ntv = valid_ntptimeval64_to_ntptimeval (ntv64); |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> https://sourceware.org/pipermail/libc-testresults/2020q2/006191.html
> 

Thanks for spotting it.

I'm wondering if those archs use different set of gcc switches for
compilation?

And another question (I think related) - after updating the the glibc
-master (there was a switch to gcc 10 for build-many-glibc.py) I do have
an issue with "check-compilers" task on those archs.

Joseph, do you use updated setup?


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-20 17:08         ` Lukasz Majewski
@ 2020-05-20 17:21           ` Joseph Myers
  2020-05-21 10:31             ` Lukasz Majewski
  0 siblings, 1 reply; 21+ messages in thread
From: Joseph Myers @ 2020-05-20 17:21 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Florian Weimer, Andreas Schwab, GNU C Library, Alistair Francis

On Wed, 20 May 2020, Lukasz Majewski wrote:

> I'm wondering if those archs use different set of gcc switches for
> compilation?

No.  But there are various architecture-specific aspects to optimization 
that may result in warnings showing up on only some architectures.

Florian has fixed this bug.

> And another question (I think related) - after updating the the glibc
> -master (there was a switch to gcc 10 for build-many-glibc.py) I do have
> an issue with "check-compilers" task on those archs.

A check-compilers failure simply means that one of the tasks from the 
"compilers" run failed.

In general, if you did a "compilers" run when the build was broken, you 
will have an incomplete set of compilers that isn't good for testing 
subsequent glibc changes and will need to rerun "compilers" with the 
source trees in an unbroken state.

> Joseph, do you use updated setup?

My bots using GCC release branches only rebuild "compilers" once a week.  
That means a short-lived glibc build breakage is likely to show up as a 
failure in "glibcs" rather than "compilers" (but if the build is broken at 
the wrong time, when "compilers" runs, the "glibcs" builds will be using 
the broken compilers for a week).

My bot using GCC master rebuilds the compilers every time (but only runs 
once a day, whereas the ones using GCC release branches will run more 
frequently if there are new glibc changes to test).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation
  2020-05-20 17:21           ` Joseph Myers
@ 2020-05-21 10:31             ` Lukasz Majewski
  0 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2020-05-21 10:31 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Florian Weimer, Andreas Schwab, GNU C Library, Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hi Joseph,

> On Wed, 20 May 2020, Lukasz Majewski wrote:
> 
> > I'm wondering if those archs use different set of gcc switches for
> > compilation?  
> 
> No.  But there are various architecture-specific aspects to
> optimization that may result in warnings showing up on only some
> architectures.
> 
> Florian has fixed this bug.
> 
> > And another question (I think related) - after updating the the
> > glibc -master (there was a switch to gcc 10 for
> > build-many-glibc.py) I do have an issue with "check-compilers" task
> > on those archs.  
> 
> A check-compilers failure simply means that one of the tasks from the 
> "compilers" run failed.
> 
> In general, if you did a "compilers" run when the build was broken,
> you will have an incomplete set of compilers that isn't good for
> testing subsequent glibc changes and will need to rerun "compilers"
> with the source trees in an unbroken state.

Yes, you are 100% correct. That was the case - I wanted to rebuild
compilers after update to gcc 10 for build-many-glibc.py.

As a result I used the broken glibc for building compilers.

Thanks for explanation.

> 
> > Joseph, do you use updated setup?  
> 
> My bots using GCC release branches only rebuild "compilers" once a
> week. That means a short-lived glibc build breakage is likely to show
> up as a failure in "glibcs" rather than "compilers" (but if the build
> is broken at the wrong time, when "compilers" runs, the "glibcs"
> builds will be using the broken compilers for a week).
> 
> My bot using GCC master rebuilds the compilers every time (but only
> runs once a day, whereas the ones using GCC release branches will run
> more frequently if there are new glibc changes to test).
> 


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-05-21 10:31 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
2020-05-15 10:03   ` Lukasz Majewski
2020-05-19 19:07   ` Adhemerval Zanella via Libc-alpha
2020-05-19 19:58     ` Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 2/7] y2038: linux: Provide ___adjtimex64 implementation Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 3/7] y2038: linux: Provide __adjtime64 implementation Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type Lukasz Majewski
2020-05-19 19:10   ` Adhemerval Zanella via Libc-alpha
2020-05-08 14:56 ` [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64 Lukasz Majewski
2020-05-19 19:12   ` Adhemerval Zanella via Libc-alpha
2020-05-08 14:56 ` [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation Lukasz Majewski
2020-05-19 19:18   ` Adhemerval Zanella via Libc-alpha
2020-05-19 20:20     ` Lukasz Majewski
2020-05-19 20:25       ` Adhemerval Zanella via Libc-alpha
2020-05-20 15:23       ` Joseph Myers
2020-05-20 17:08         ` Lukasz Majewski
2020-05-20 17:21           ` Joseph Myers
2020-05-21 10:31             ` Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation Lukasz Majewski
2020-05-19 19:19   ` Adhemerval Zanella via Libc-alpha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).