On Mon, Apr 22, 2019 at 11:07 AM Stepan Golosunov
<ste...@golosunov.pp.ru> wrote:
> 20.04.2019 в 13:21:12 +0200 Lukasz Majewski написал:
> Is it? The kernel (5.1-rc6) code looks to me like
>
> /* Zero out the padding for 32 bit systems or in compat mode */
> if (false && false)
> kts.tv_nsec &= 0xFFFFFFFFUL;
>
> in 32-bit kernels. And like
>
> if (false && true)
> kts.tv_nsec &= 0xFFFFFFFFUL;
>
> for COMPAT syscalls in 64-bit kernels.
>
> It should probably be changed into
>
> if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall())
> kts.tv_nsec &= 0xFFFFFFFFUL;
>
> (Or into something like
>
> if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall() &&
> !COMPAT_USE_64BIT_TIME)
> kts.tv_nsec &= 0xFFFFFFFFUL;
>
> if x32 should retain 64-bit tv_nsec.)
I think the problem is that at some point CONFIG_64BIT_TIME was
meant to be enabled on both 32-bit and 64-bit kernels, but the
definition got changed along the way.
We probably just want
if (in_compat_syscall() )
kts.tv_nsec &= 0xFFFFFFFFUL;
here, which would then truncate the nanoseconds for all compat
mode including x32. For native mode, we don't need to truncate
it, since timespec64 has a 32-bit 'tv_nsec' field in the kernel.
> > However, I would prefer not to pass random data
> > to the kernel, and hence I do clear it up explicitly in glibc.
>
> If the kernel does not ignore padding on its own, then zeroing it out
> is required everywhere timespec is passed to kernel, including via
> code not known to glibc. (Does anyone promise that there won't be any
> ioctls that accept timespec, for example?) That seems to be
> error-prone (and might requre copying larger structes).
>
> On the other hand, if kernel 5.1+ ignores padding as intended there is
> no need to create additional copy of structs in glibc code that calls
> into clock_settime64 (or into timer_settime64 that accepts larger
> struct, for example).
The intention is that the kernel ignores the padding. If you find
another place in the kernel that forget that, we should fix it.
> > > And, hmm, is CONFIG_64BIT_TIME enabled anywhere?
>
> I guess that the remaining CONFIG_64BIT_TIME in kernel should be
> replaced with CONFIG_COMPAT_32BIT_TIME or removed.
We should remove CONFIG_64BIT_TIME. CONFIG_COMPAT_32BIT_TIME
is still needed to identify architectures that don't have it, in
particular riscv32.
Arnd
