Re: [PATCH 3/6] y2038: linux: Provide __clock_settime64 implementation

[Lukasz Majewski <lukma at denx dot de>]

Hi Joseph,

> On Mon, 15 Apr 2019, Lukasz Majewski wrote:
> 
> > The new 64 bit syscall (clock_settime64) available from Linux
> > 5.1+ has been used when applicable on 32 bit systems.
> > The execution path on 64 bit systems has not been changed or
> > affected in any way.  
> 
> Is this unchanged specifically because of __TIMESIZE conditionals in
> the code (so that it doesn't matter whether the 64-bit systems define
> any new syscall names)?

I think yes.

When __TIMESIZE == 64 the clock_settime syscall is supporting 64 bit
time. The goal is to not change execution path for e.g. x86_64.
IMHO this shall be kept separate from Y2038 and 32 bit execution path.


This patch tries to follow 64 bit conversion for __clock_* functions as
presented in:
https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29

__TIMESIZE is necessary to distinct the 32/64 bit case as
__clock_settime64 is used for both cases.

In this function I also need to check if __TIMESIZE is defined
https://github.com/lmajewski/y2038_glibc/commit/80f3b6220d80a8579e88b63fb90ba8ea7b771c5c#diff-725737619c471831ff796099fbab5c0aR33

as __clock_settime64 is aliased to clock_settime on 64 bit systems and
then aliased/linked to librt (for backward compatibility).

> 
> Also, I don't see any __ASSUME_* / ENOSYS handling in this patch.  As 
> usual, for any new syscall that might be used in code that could also
> use an older syscall:

I've followed some code pattern from glibc sources; for example:
sysdeps/unix/sysv/linux/lseek.c
Here the new syscall is only guarded by: # ifdef __NR__llseek
There was no __ASSUME_* for it.


However, the statx for example uses __ASSUME_STATX, which is defined
when kernel is newer than specified version. I will rewrite the code to
be similar to statx.

> 
> * If the appropriate __ASSUME_* is defined, the new syscall can
> simply be used unconditionally.
> 
> * If the appropriate __ASSUME_* is not defined, there needs to be
> runtime fallback to the old syscall if the new one returns an ENOSYS
> error (of course, that runtime fallback needs to check for the time
> overflowing the 32-bit range and give an appropriate error in that
> case, before calling the 32-bit syscall - and it's necessary to
> figure out what the priority should be of errors for invalid
> nanoseconds values versus overflowing seconds).  It is normal and
> expected for the kernel headers used to build glibc to be (much)
> newer than the oldest kernel version supported by the resulting glibc
> binaries at runtime.

Ach... I see your point. The __ASSUME_ prevents from nonfunctional glibc
when one uses new headers to compile glibc (which have
__NR__clock_settime64 defined) afterwards installed in a system with
old kernel (so __clock_settime64 returns -ENOSYS). I do agree that with
current patch there is no fallback to old syscall in that case.

I will fix it in v2.

> 
> This __ASSUME_* macro definition in kernel-features.h would need a 
> multi-paragraph comment discussing exactly what the semantics of such 
> __ASSUME_* macros are in the context of the sets of syscall names and 
> numbers present on different kinds of Linux kernel architectures.

Ok. I will add it to v2.

> 
> > +#if __TIMESIZE != 64
> > +int
> > +__clock_settime (clockid_t clock_id, const struct timespec *tp)
> > +{
> > +  struct __timespec64 ts64;
> > +
> > +  if (! in_time_t_range (tp->tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }  
> 
> I don't see how an overflow check makes sense in this context
> (converting a 32-bit timespec to a 64-bit one).

The above code is for the case when _TIME_BITS is not set (on 32
bit system) and we are after Y2038. The data provided by *tp has wrong
value and shall not be passed to the kernel (no matter if it supports
Y2038 time or not).

> 




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: pgpUgJ0Xw6jVP.pgp
Description: OpenPGP digital signature