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

From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Deepa Dinamani <deepa.kernel@gmail.com>,
	tglx@linutronix.de, john.stultz@linaro.org
Cc: y2038@lists.linaro.org, linux-kernel@vger.kernel.org, arnd@arndb.de
Subject: Re: [Y2038] [PATCH v2 08/10] fix get_timespec64() for y2038 safe compat interfaces
Date: Fri, 15 Dec 2017 00:21:50 +0000	[thread overview]
Message-ID: <1513297310.18523.293.camel@codethink.co.uk> (raw)
In-Reply-To: <20171127193037.8711-9-deepa.kernel@gmail.com>

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.