Re: 32-bit time_t inside itimerval

[Alistair Francis <alistair23 at gmail dot com>]

On Sat, Dec 21, 2019 at 5:31 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Dec 20, 2019 at 10:35 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > Hey,
> >
> > I just noticed something strange.
> >
> > The setitimer syscall is not different for 32/64-bit time_t. So we use
> > this syscall for both 32/64 time_t [1]
> >
> > SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
> >         struct itimerval __user *, ovalue)
> >
> > Where struct itimerval __user *, value looks like this [2]:
> >
> > struct itimerval {
> >     struct timeval it_interval;    /* timer interval */
> >     struct timeval it_value;    /* current value */
> > };
> >
> > Which then uses this structure for timeval [3]:
> >
> > struct timeval {
> >     __kernel_old_time_t    tv_sec;        /* seconds */
> >     __kernel_suseconds_t    tv_usec;    /* microseconds */
> > };
> >
> > And __kernel_old_time_t is defined [4] as:
> >
> > typedef __kernel_long_t    __kernel_old_time_t;
> > typedef __kernel_long_t    __kernel_time_t;
>
> Right, with my latest patches, this gets changed to __kernel_old_timeval, with
> an unmodified definititon.
>
> > So __kernel_old_time_t and __kernel_time_t are both 32-bit values
> > inside the kernel. and the setiimer syscall expects a 32-bit time_t
> > for the values inside it.
> >
> > This is different to the current glibc implementation where all time_t
> > values are treated as 64-bit for RISC-V [5].
> >
> > What should we do here?
> >
> > 1. Change glibc to use a 32-bit time_t for some structures, such as
> > the timeval inside itimerval?
> > 2. Convert the kernel time_t to be 64-bit for RV32?
>
> What happened here is that originally I thought we would not need
> setitimer/getitimer
> and could fall back to timer_settime/timer_gettime, but that turned out to be a
> misunderstanding (we do need both).
>
> By the time we introduced all the other system calls with 64-bit
> time_t in linux-5.1,
> there was no replacement yet, but since these interfaces never pass
> absolute times
> on the kernel ABI, it was considered good enough. There was a small debate on
> whether struct itimerval and struct rusage (which has the same problem) should
> have replacements using struct __kernel_timespec, or a newly added
> __kernel_timeval, and that discussion never had a conclusion, so we
> left it at this.

Thanks for clarifying ths Arnd.

Ok, I didn't realise this was the case. It ends up being a bit of a pain.

>
> For glibc, the only sensible implementation is to implement the time64
> settimer/getitimer interfaces on top of the time32 setitimer/getitimer
> system calls,
> doing the conversion internally. (Same for getrusage and wait4).

Ok, so we need to fix setitimer/getitimer,  getrusage and waitid's
rusage (wait4 isn't in y2038 safe calls).

For the glibc people, can we do something like this?

 1. Add a __old_timeval struct used by the itimerval and rusage structs
 2. Make __old_timeval use __old_time_t that is always a long (no
matter what time_t really is)

Then the question becomes do we expose __old_timeval (with 32-bit
time_t) or the real timeval (64-bit time_t) to callers of the
functions?

If we are exposing the new one we will have to edit the four functions
to convert 32-bit to 64-bit. Which should all be pretty straight
forward.
If we don't expose the new one it should be even easier.

I'm not sure what is more appropriate for glibc, let me know and I
should be able to get some patches ready. Maybe this can make it in
before freeze :)

>
> We may still want to introduce getitimer_time64, setiitimer_timer64,
> getrusage_time64 and waitid_time64 at some point, using __kernel_timespec
> to have a saner user space interface, but there is no real point in glibc
> using those syscalls as the underlying implementation when the fallback
> to the time32 versions is still required.

I would +1 adding getitimer_time64, setiitimer_timer64,
getrusage_time64 and waitid_time64 as it simplifies things.

Alistair

>
>         Arnd