Re: [Y2038] [PATCH v2 07/10] include: Add new y2038 safe __kernel_timespec

[Ben Hutchings]

On Mon, 2017-11-27 at 11:30 -0800, Deepa Dinamani wrote:
> The new struct __kernel_timespec is similar to current
> internal kernel struct timespec64 on 64 bit architecture.
> The compat structure however is similar to below on little
> endian systems (padding and tv_nsec are switched for big
> endian systems):
> 
> typedef s32ÂÂÂÂÂÂÂÂÂÂÂÂcompat_long_t;
> typedef s64ÂÂÂÂÂÂÂÂÂÂÂÂcompat_kernel_time64_t;
> 
> struct compat_kernel_timespec {
> ÂÂÂÂÂÂÂcompat_kernel_time64_tÂÂtv_sec;
> ÂÂÂÂÂÂÂcompat_long_tÂÂÂÂÂÂÂÂÂÂÂtv_nsec;
> ÂÂÂÂÂÂÂcompat_long_tÂÂÂÂÂÂÂÂÂÂÂpadding;
> };
> 
> This allows for both the native and compat representations to
> be the same and syscalls using this type as part of their ABI
> can have a single entry point to both.
> 
> Note that the compat define is not included anywhere in the
> kernel explicitly to avoid confusion.

If I understand correctly, the intent here is that C libraries will be
allowed to define struct timespec like that when appropriate feature
macros are enabled.  Could you spell that out in the commit message,
and also the need to clear padding on the kernel side?

[...]
> --- a/include/uapi/linux/time.h
> +++ b/include/uapi/linux/time.h
> @@ -42,6 +42,13 @@ struct itimerval {
> > > Â	struct timeval it_value;	/* current value */
> Â};
> Â
> +#ifndef __kernel_timespec
> +struct __kernel_timespec {
> +	__kernel_time64_tÂÂÂÂÂÂÂtv_sec;ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ/* seconds */
> +	long longÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂtv_nsec;ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ/* nanoseconds */
> +};
> +#endif

I wonder if it makes sense to override the alignment of this structure?
 (64-bit types are aligned differently on 32-bit vs 64-bit x86, but not
other compat cases.)  It might reduce the need for conversions in
compat code elsewhere later.

Ben.

> Â/*
> Â * The IDs of the various system clocks (for POSIX.1b interval timers):
> Â */
-- 
Ben Hutchings
Software Developer, Codethink Ltd.