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]

Re: [Y2038] Fifth draft of the Y2038 design document


On Mon, Feb 27, 2017 at 12:02 PM, Albert ARIBAUD
<albert.aribaud@3adev.fr> wrote:
> Hi Arnd,
>
> On Thu, 23 Feb 2017 21:24:22 +0100, Arnd Bergmann <arnd@arndb.de>
> wrote :
>
>> On Thu, Feb 23, 2017 at 6:31 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> > On Thu, 23 Feb 2017, Arnd Bergmann wrote:
>> >
>>  [...]
>> >>
>> >> I had thought of that as well, but wasn't sure if we could rely on bitfields
>> >> to do the right thing across C standard versions and non-gcc compilers.
>> >> I experimentally found that gcc-4.9 and higher accept that version without
>> >> a -Wpedantic warning when using --std=c99 or --std=c11, but they warn
>> >> about it with --std=c89 or --std=gnu89. Older gcc versions as far back as
>> >> gcc-3.4 (the oldest I could try) always warn about anonymous bitfields
>> >> with -pedantic, but I could still silence that warning using the
>> >> __extension__ keyword. I think that's good enough.
>> >
>> > What's the warning you see, with what exact testcase?  Unnamed bit-fields
>> > (provided the declared type is int / signed int / unsigned int, not any
>> > other integer or enum type), and initializers ignoring them, date back at
>> > least to the third public review draft of C89 (13 May 1988), which is the
>> > oldest version I have to hand.
>>
>> Sorry, my mistake, I used "long : 32", which causes "warning: type of
>> bit-field '<anonymous>' is a GCC extension [-Wpedantic]". With 'int'
>> it works fine.
>
> So IIUC (and assuming we keep tv_nsec a long in APIs)  we /could/ pad
> struct timespec with an anonymous bitfield and still be compatible with
> (most) existing user source code, and it is considered acceptable (but
> should be duly documented). Added this in rev. 122of the design doc.

This solves the problem in user space, but it still leaves the other problem
that Zack and I mentioned on the user/kernel boundary: we have to zero
out the padding since at least 64-bit kernels with 32-bit user space
would interpret the padding as nanoseconds over 1 billion. We may also
want to make the kernel behavior consistent between 32-bit and 64-bit
kernels, so the kernel will either always ignore the padding (which is
a bit more work for the kernel) or it will always reject nonzero upper bits
of the nanoseconds (as 64-bit kernels do today for native tasks, this
means more work in 32-bit glibc, or using 64-bit nanoseconds as
Zack suggested).

Right now, I have this in the kernel, to do the zeroing whenever we
copy a __kernel_timespec from user space into a timespec64
in the kernel:

typedef __s64 __kernel_time64_t;

struct __kernel_timespec {
        __kernel_time64_t       tv_sec;
        __s64                   tv_nsec;
};

int get_timespec64(struct timespec64 *ts,
                   const struct __kernel_timespec __user *uts)
{
        struct __kernel_timespec kts;
        int ret;

        ret = copy_from_user(&kts, uts, sizeof(kts));
        if (ret)
                return -EFAULT;

        ts->tv_sec = kts.tv_sec;
        if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall())
                kts.tv_nsec &= 0xfffffffful;
        ts->tv_nsec = kts.tv_nsec;

        return 0;
}

This should catch anything that passes in a timespec by itself
(clock_settime, clock_nanosleep, timer_settime, timerfd_settime,
ppoll, pselect6, io_getevents, recvmmsg, mq_timedsend,
mq_timedreceive, semtimedop, utimensat, rt_sigtimedwait,
futex and some ioctls), but not structures that embed a timespec
(fortunately no syscalls, very few ioctls). The external function
call plus the in_compat_syscall() check will add a couple of cycles
on 64-bit architectures, for both 32-bit and 64-bit tasks. We could
optimize this slightly using

    if (IS_ENABLED(CONFIG_64BIT))
                 kts.tv_nsec &= current->tv_nsec_mask;

      Arnd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]