This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Arnd, > On Tue, Jun 25, 2019 at 5:51 PM Lukasz Majewski <lukma@denx.de> wrote: > > > On Tue, Jun 25, 2019 at 2:11 AM Alistair Francis > > > <alistair.francis@wdc.com> wrote: > > > > > > > weak_alias (__clock_settime, clock_settime) > > > > + > > > > +#if __TIMESIZE != 64 > > > > +int > > > > +__clock_settime (clockid_t clock_id, const struct timespec *tp) > > > > +{ > > > > + struct __timespec64 ts64; > > > > + > > > > + valid_timespec_to_timespec64 (tp, &ts64); > > > > + return __clock_settime64 (clock_id, &ts64); > > > > +} > > > > +#endif > > > > > > I missed this when Lukasz first posted this, but I would still > > > prefer this to be changed. > > > > > > Having clock_settime() (using the weak_alias) call into > > > __clock_settime64() means that a kernel that warns about > > > old user space calling into the old syscall never notices this. > > > > Could you be more specific here? > > > > > > I thought that we do not care about the legacy systems (i.e. those > > which don't switch to 64 bit time syscalls before Y2038). > > > > The assumption is to not break anything and use 64 bit time related > > syscalls after v5.1 kernel (i.e. clock_settime64 and friends on > > systems with __WORDSIZE=32). > > > > Syscalls, which handle only 32 bit time would be used as fallback. > > I don't mind falling back on the 32-bit implementation from a time64 > syscall when running on the old kernel, that part is required to make > new binaries run on pre-5.1 kernel. > > Your __clock_settime however does the reverse: you have an application > that calls clock_settime(), the alias redirects that to > __clock_settime(), and that converts it into the 64-bit structure and > passes it into __clock_settime64(), which then calls the time64 > syscall before falling back to calling the time32 syscall. This patch focuses on the situation where we strive to provide/use 64 bit time support and try not to rely on 32 bit interfaces. > > This is problematic in the scenario that you have an embedded system > you deploy today, and turn off the time32 syscalls in the kernel. I assume that then we would only have __NR_clock_settime64 defined (no __NR_clock_settime available) on WORDSIZE==32 archs? > All applications are built against the time64 glibc interfaces, except > for one tool that someone forgot. This calls the old clock_settime() > with a 32-bit time, which gets converted into and passed into > clock_settime64 in the kernel where it successfully sets the time at > boot. Interesting use case. Thanks for sharing it. > > In 2038, it stops working because of the time_t overflow that was > not caught during validation. If we call the time32 interface here, it > breaks immediately on kernels that return -ENOSYS from > clock_gettime(), Maybe I'm not aware of something, but isn't the removal of clock_settime syscall supporting only 32 bit time (on archs with WORDSIZE==32) the ABI break? Shouldn't those syscalls be kept until the minimal supported glibc kernel version is 5.1? > which makes the validation easier and more reliable. > > > > Can you please leave the existing clock_settime() > > > > I think that some pseudo code would shed some more light on your > > idea. > > What I mean is to have > > #ifdef __NR_clock_settime > int > __clock_settime (clockid_t clock_id, const struct timespec *tp) > { > return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); > } > #endif > > This will do the right thing on 64-bit architectures (which use time64 > always), on new 32-bit architectures (not provide __clock_settime > at all, clock_settime() should be an alias for __clock_settime64()) > and on old 32-bit architectures (work as expected on existing > kernels, fail with -ENOSYS when we want it to). > The latest patch for clock_settime [1]: Could be changed to: #if __TIMESIZE != 64 int __clock_settime (clockid_t clock_id, const struct timespec *tp) { /* For WORDSIZE==32 systems the headers could still have defined __NR_clock_settime, but the kernel itself may not support it anymore */ #ifdef __NR_clock_settime return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); #endif struct __timespec64 ts64; valid_timespec_to_timespec64 (tp, &ts64); return __clock_settime64 (clock_id, &ts64); } #endif However, there is the problem that in some point in time the glibc will switch to 64 bit __TIMESIZE only (probably when minimal kernel version for glibc would be grater than 5.1) and all __clock_settime syscalls would be served with __clock_settime64 (as 64 bit time support is always in place). After this switch the "unconverted" program will setup wrong time. > > and most notably from: > > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 > > recommended by glibc for the syscalls conversion for 64 bit. > > > > It also uses the __ASSUME_TIME64_SYSCALLS flag. > > I thought it was covered in there, but maybe that changed. This flag is supposed to indicate if the kernel supports 64 bit time interface (either with clock_settime syscall on WORDSIZE==64 or clock_settime64 on WORDSIZE==32, including x32). The newest proposition for it: [2] > > Arnd Note: [1] - https://patchwork.ozlabs.org/patch/1107235/ [2] - https://patchwork.ozlabs.org/patch/1117100/ 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:
pgpsfpoCs6L9W.pgp
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |