Re: [Y2038] [PATCH v2 08/10] fix get_timespec64() for y2038 safe compat interfaces

[Ben Hutchings <ben.hutchings@codethink.co.uk>]

On Mon, 2017-11-27 at 11:30 -0800, Deepa Dinamani wrote:
> get/put_timespec64() interfaces will eventually be used for
> conversions between the new y2038 safe struct __kernel_timespec
> and struct timespec64.
> 
> The new y2038 safe syscalls have a common entry for native
> and compat interfaces.
> On compat interfaces, the high order bits of nanoseconds
> should be zeroed out. This is because the application code
> or the libc do not garuntee zeroing of these. If used without

Spelling: "guarantee"

[...]
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
[...]
> @@ -851,6 +851,11 @@ int get_timespec64(struct timespec64 *ts,
>  		return -EFAULT;
>  
>  	ts->tv_sec = kts.tv_sec;
> +
> +	/* Zero out the padding for 32 bit systems or in compat mode */
> +	if (IS_ENABLED(CONFIG_64BIT_TIME) || !IS_ENABLED(CONFIG_64BIT) || in_compat_syscall())
> +		kts.tv_nsec &= 0xFFFFFFFFUL;
[...]

I don't understand the condition here.  Suppose we're building for an
architecture that enables the new syscalls and selects ARCH_64BIT_TIME,
but we also enable 64BIT.  Then the above condition ends up as:
	if (1 || 0 || in_compat_syscall())
so it's always true.

Should the condition be:
	if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall())
or is your intent that architectures only select ARCH_64BIT_TIME if
64BIT is not enabled?

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.