Message ID | 20191206231004.10380-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [1/2] y2038: linux: Provide __timerfd_gettime64 implementation | expand |
Dear Glibc Community, I would like to share the current state of development and potentially avoid effort duplication. Please find the status update info regarding the effort to make glibc supporting 64 bit time on systems with TIMESIZE != 64 (i.e. 32 bit ARM). The most up to date setup - with "upstream first" philosophy applied (including patch to provide support for -D_TIME_BITS=64) to glibc can be found at [1]. Following functions to support 64 bit syscalls have been converted: - clock_gettime64 - clock_settime64 - clock_getres64 - utimensat - futimens - clock_nanosleep_time64 - ppoll64 - timer_gettime - timer_settime - timerfd_gettime (WIP) - timerfd_settime (WIP) [2] - pselect64 conversion has been postponed until we advance with minimal supported glibc version (now it is 3.2 , we need to be > 3.15). After 3.15 it would be far more easier to do the conversion. It is possible to test this effort with QEMU and OE/Yocto. Detailed tutorial in README file [3]. TO DO: 0. Use other 64 bit syscalls - as indicated in [4]. 1. Convert in-glibc usage of internal clock functions (e.g. __clock_gettime()) to __clock_gettime64(), which are Y2038 safe on 32 bit systems with TIMESIZE==32. Potential issue - making those functions as hidden and in the same time accessible by Y2038 safe systems (e.g. __clock_gettime64()). 2. The statx (and friends) conversion to Y2038 (Alistair also did some work there) 3. Continue replacing Y2038 unsafe functions (like gettimeofday) with internal, safe representation (which would use __clock_gettime64()) as was done by Zack and Adhemerval. 4. Prepare for "big switch" to add support for -D_TIME_BITS=64 in glibc - I'm mostly concerned with preparing a solid test sute. Please do not hesitate to provide input about any other ongoing work in this area. Please also forward this mail to any potentially interested parties. Last, but not least, I would like to give a big thank to all community members involved into the development and review process. Links: [1] - https://github.com/lmajewski/y2038_glibc/commits/glibc_timerfd_settime_gettime-conversion-v1 [2] - posted for review: https://patchwork.ozlabs.org/patch/1205307/ [3] - https://github.com/lmajewski/meta-y2038 [4] - https://elixir.bootlin.com/linux/v5.3-rc5/source/arch/arm/tools/syscall.tbl#L420 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
On Sat, 7 Dec 2019, Lukasz Majewski wrote: > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c b/sysdeps/unix/sysv/linux/timerfd_gettime.c > new file mode 100644 > index 0000000000..498605369b > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c > @@ -0,0 +1,69 @@ > +/* Copyright (C) 2003-2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. New files need to have a descriptive comment before the copyright notice, and no "Contributed by" line, and only have previous years in the copyright notice if genuinely incorporating copyrightable content from previous files from those years. > +int > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) > +{ > + if (fd < 0) > + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EBADF); Why? In general, such checks are only needed in userspace if correct function semantics means not passing such a case to the kernel at all, as opposed to letting the kernel return an error for it. The same comments apply to patch 2/2.
Hi Joseph, > On Sat, 7 Dec 2019, Lukasz Majewski wrote: > > > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c > > b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644 > > index 0000000000..498605369b > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c > > @@ -0,0 +1,69 @@ > > +/* Copyright (C) 2003-2019 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. > > New files need to have a descriptive comment before the copyright > notice, and no "Contributed by" line, and only have previous years in > the copyright notice if genuinely incorporating copyrightable content > from previous files from those years. Thanks for the hint. I will add the descriptive comment and treat this file as a "new" one - with copyright description similar to newly added files. > > > +int > > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) > > +{ > > + if (fd < 0) > > + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EBADF); ^^^^^^^^^ - [*] > > Why? In general, such checks are only needed in userspace if correct > function semantics means not passing such a case to the kernel at > all, as opposed to letting the kernel return an error for it. I took this approach from futimens.c (from sysdeps/unix/sysv/linux). This is how the wrong fd is handled. According to manual [1] the EBADF shall be returned. The manual for timerfd_gettime [2] also states that wrong fd shall cause the EBADF return value. And yes - you are right - the Linux kernel checks [3] if fd is valid and returns either -EBADF or -EINVAL. Considering the above - I will remove the check [*] from __timerfd_gettime64 and send v2. > > The same comments apply to patch 2/2. > Ok. Links: [1] - https://linux.die.net/man/3/futimens [2] - https://linux.die.net/man/2/timerfd_gettime [3] - https://elixir.bootlin.com/linux/v5.4/source/fs/timerfd.c#L374 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
diff --git a/include/time.h b/include/time.h index e5e8246eac..eb5082b4d7 100644 --- a/include/time.h +++ b/include/time.h @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64); #if __TIMESIZE == 64 # define __timer_gettime64 __timer_gettime +# define __timerfd_gettime64 __timerfd_gettime #else extern int __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value); +extern int __timerfd_gettime64 (int fd, struct __itimerspec64 *value); libc_hidden_proto (__timer_gettime64); +libc_hidden_proto (__timerfd_gettime64); #endif #if __TIMESIZE == 64 diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 07776d28ea..e599fcec23 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -18,7 +18,8 @@ sysdep_routines += adjtimex clone umount umount2 readahead \ setfsuid setfsgid epoll_pwait signalfd \ eventfd eventfd_read eventfd_write prlimit \ personality epoll_wait tee vmsplice splice \ - open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get + open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \ + timerfd_gettime CFLAGS-gethostid.c = -fexceptions CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index 603e517ca6..385007c662 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -95,7 +95,6 @@ mq_setattr - mq_getsetattr i:ipp mq_setattr timerfd_create EXTRA timerfd_create i:ii timerfd_create timerfd_settime EXTRA timerfd_settime i:iipp timerfd_settime -timerfd_gettime EXTRA timerfd_gettime i:ip timerfd_gettime fanotify_init EXTRA fanotify_init i:ii fanotify_init diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644 index 0000000000..498605369b --- /dev/null +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c @@ -0,0 +1,69 @@ +/* Copyright (C) 2003-2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. + + 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 <kernel-features.h> + +int +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) +{ + if (fd < 0) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EBADF); + +#ifdef __ASSUME_TIME64_SYSCALLS +# ifndef __NR_timerfd_gettime64 +# define __NR_timerfd_gettime64 __NR_timerfd_gettime +# endif + return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); +#else +# ifdef __NR_timerfd_gettime64 + int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); + if (ret == 0 || errno != ENOSYS) + return ret; +# endif + struct itimerspec its32; + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); + if (retval == 0) + { + value->it_interval = valid_timespec_to_timespec64 (its32.it_interval); + value->it_value = valid_timespec_to_timespec64 (its32.it_value); + } + + return retval; +#endif +} + +#if __TIMESIZE != 64 +int +__timerfd_gettime (int fd, struct itimerspec *value) +{ + struct __itimerspec64 its64; + int retval = __timerfd_gettime64 (fd, &its64); + if (retval == 0) + { + value->it_interval = valid_timespec64_to_timespec (its64.it_interval); + value->it_value = valid_timespec64_to_timespec (its64.it_value); + } + + return retval; +} +#endif +strong_alias (__timerfd_gettime, timerfd_gettime)