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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1513297310.18523.293.camel@codethink.co.uk>
Date:   Fri, 15 Dec 2017 00:21:50 +0000
From:   Ben Hutchings <ben.hutchings@...ethink.co.uk>
To:     Deepa Dinamani <deepa.kernel@...il.com>, tglx@...utronix.de,
        john.stultz@...aro.org
Cc:     y2038@...ts.linaro.org, linux-kernel@...r.kernel.org, arnd@...db.de
Subject: Re: [Y2038] [PATCH v2 08/10] fix get_timespec64() for y2038 safe
 compat interfaces

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.