linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
@ 2019-04-29 13:19 Arnd Bergmann
  2019-04-29 13:19 ` [PATCH 2/2] y2038: remove CONFIG_64BIT_TIME Arnd Bergmann
  2019-05-09  8:52 ` [tip:timers/urgent] y2038: Make CONFIG_64BIT_TIME unconditional tip-bot for Arnd Bergmann
  0 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-29 13:19 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Joseph Myers, libc-alpha, linux-api, linux-kernel,
	Deepa Dinamani, Arnd Bergmann, Lukasz Majewski, Stepan Golosunov

As Stepan Golosunov points out, we made a small mistake in the
get_timespec64() function in the kernel. It was originally added under
the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
and 64-bit architectures, but when I did the conversion, I only turned
it on for 32-bit ones.

The effect is that the get_timespec64() function never clears the upper
half of the tv_nsec field for 32-bit tasks in compat mode. Clearing this
is required for POSIX compliant behavior of functions that pass a
'timespec' structure with a 64-bit tv_sec and a 32-bit tv_nsec, plus
uninitialized padding.

The easiest fix for linux-5.1 is to just make the Kconfig symbol
unconditional, as it was originally intended. As a follow-up,
we should remove any #ifdef CONFIG_64BIT_TIME completely.

Note: for native 32-bit mode, no change is needed, this works as
designed and user space should never need to clear the upper 32
bits of the tv_nsec field, in or out of the kernel.

Fixes: 00bf25d693e7 ("y2038: use time32 syscall names on 32-bit")
Link: https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Stepan Golosunov <stepan@golosunov.pp.ru>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Please apply this one as a bugfix for 5.1
---
 arch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 33687dddd86a..9092e0ffe4d3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
 	bool
 
 config 64BIT_TIME
-	def_bool ARCH_HAS_64BIT_TIME
+	def_bool y
 	help
 	  This should be selected by all architectures that need to support
 	  new system calls with a 64-bit time_t. This is relevant on all 32-bit
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] y2038: remove CONFIG_64BIT_TIME
  2019-04-29 13:19 [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional Arnd Bergmann
@ 2019-04-29 13:19 ` Arnd Bergmann
  2019-05-09  8:52 ` [tip:timers/urgent] y2038: Make CONFIG_64BIT_TIME unconditional tip-bot for Arnd Bergmann
  1 sibling, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-29 13:19 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Joseph Myers, libc-alpha, linux-api, linux-kernel,
	Deepa Dinamani, Arnd Bergmann, Lukasz Majewski, Stepan Golosunov

The CONFIG_64BIT_TIME option is defined on all architectures, and can
be removed for simplicity now.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This would make sense for 5.2, or could be part of a later
cleanup series when I have more patches for the remaining
y2038 bits
---
 arch/Kconfig          | 8 --------
 fs/aio.c              | 2 +-
 ipc/syscall.c         | 2 +-
 kernel/time/hrtimer.c | 2 +-
 kernel/time/time.c    | 4 ++--
 net/socket.c          | 2 +-
 6 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 9092e0ffe4d3..23ee740182aa 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -763,14 +763,6 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
-config 64BIT_TIME
-	def_bool y
-	help
-	  This should be selected by all architectures that need to support
-	  new system calls with a 64-bit time_t. This is relevant on all 32-bit
-	  architectures, and 64-bit architectures as part of compat syscall
-	  handling.
-
 config COMPAT_32BIT_TIME
 	def_bool !64BIT || COMPAT
 	help
diff --git a/fs/aio.c b/fs/aio.c
index 3490d1fa0e16..b1b949ae1a93 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2057,7 +2057,7 @@ static long do_io_getevents(aio_context_t ctx_id,
  *	specifies an infinite timeout. Note that the timeout pointed to by
  *	timeout is relative.  Will fail with -ENOSYS if not implemented.
  */
-#if !defined(CONFIG_64BIT_TIME) || defined(CONFIG_64BIT)
+#ifdef CONFIG_64BIT
 
 SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
 		long, min_nr,
diff --git a/ipc/syscall.c b/ipc/syscall.c
index 581bdff4e7c5..dfb0e988d542 100644
--- a/ipc/syscall.c
+++ b/ipc/syscall.c
@@ -30,7 +30,7 @@ int ksys_ipc(unsigned int call, int first, unsigned long second,
 		return ksys_semtimedop(first, (struct sembuf __user *)ptr,
 				       second, NULL);
 	case SEMTIMEDOP:
-		if (IS_ENABLED(CONFIG_64BIT) || !IS_ENABLED(CONFIG_64BIT_TIME))
+		if (IS_ENABLED(CONFIG_64BIT))
 			return ksys_semtimedop(first, ptr, second,
 			        (const struct __kernel_timespec __user *)fifth);
 		else if (IS_ENABLED(CONFIG_COMPAT_32BIT_TIME))
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 41dfff23c1f9..61f03faf783a 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1749,7 +1749,7 @@ long hrtimer_nanosleep(const struct timespec64 *rqtp,
 	return ret;
 }
 
-#if !defined(CONFIG_64BIT_TIME) || defined(CONFIG_64BIT)
+#ifdef CONFIG_64BIT
 
 SYSCALL_DEFINE2(nanosleep, struct __kernel_timespec __user *, rqtp,
 		struct __kernel_timespec __user *, rmtp)
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 74105fa3ce80..a4c72577aa92 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -264,7 +264,7 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct old_timeval32 __user *, tv,
 }
 #endif
 
-#if !defined(CONFIG_64BIT_TIME) || defined(CONFIG_64BIT)
+#ifdef CONFIG_64BIT
 SYSCALL_DEFINE1(adjtimex, struct __kernel_timex __user *, txc_p)
 {
 	struct __kernel_timex txc;		/* Local copy of parameter */
@@ -871,7 +871,7 @@ int get_timespec64(struct timespec64 *ts,
 	ts->tv_sec = kts.tv_sec;
 
 	/* Zero out the padding for 32 bit systems or in compat mode */
-	if (IS_ENABLED(CONFIG_64BIT_TIME) && in_compat_syscall())
+	if (in_compat_syscall())
 		kts.tv_nsec &= 0xFFFFFFFFUL;
 
 	ts->tv_nsec = kts.tv_nsec;
diff --git a/net/socket.c b/net/socket.c
index a180e1a9ff23..2ff80e03d97b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2787,7 +2787,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
 				    a[2], true);
 		break;
 	case SYS_RECVMMSG:
-		if (IS_ENABLED(CONFIG_64BIT) || !IS_ENABLED(CONFIG_64BIT_TIME))
+		if (IS_ENABLED(CONFIG_64BIT))
 			err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
 					     a[2], a[3],
 					     (struct __kernel_timespec __user *)a[4],
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:timers/urgent] y2038: Make CONFIG_64BIT_TIME unconditional
  2019-04-29 13:19 [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional Arnd Bergmann
  2019-04-29 13:19 ` [PATCH 2/2] y2038: remove CONFIG_64BIT_TIME Arnd Bergmann
@ 2019-05-09  8:52 ` tip-bot for Arnd Bergmann
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot for Arnd Bergmann @ 2019-05-09  8:52 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, linux-kernel, arnd, hpa, deepa.kernel, joseph, lukma,
	stepan, mingo

Commit-ID:  f3d964673b2f1c5d5c68c77273efcf7103eed03b
Gitweb:     https://git.kernel.org/tip/f3d964673b2f1c5d5c68c77273efcf7103eed03b
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Mon, 29 Apr 2019 15:19:37 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 29 Apr 2019 21:07:10 +0200

y2038: Make CONFIG_64BIT_TIME unconditional

As Stepan Golosunov points out, there is a small mistake in the
get_timespec64() function in the kernel. It was originally added under the
assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit and
64-bit architectures, but when the conversion was done, it was only turned
on for 32-bit ones.

The effect is that the get_timespec64() function never clears the upper
half of the tv_nsec field for 32-bit tasks in compat mode. Clearing this is
required for POSIX compliant behavior of functions that pass a 'timespec'
structure with a 64-bit tv_sec and a 32-bit tv_nsec, plus uninitialized
padding.

The easiest fix for linux-5.1 is to just make the Kconfig symbol
unconditional, as it was originally intended. As a follow-up, the #ifdef
CONFIG_64BIT_TIME can be removed completely..

Note: for native 32-bit mode, no change is needed, this works as
designed and user space should never need to clear the upper 32
bits of the tv_nsec field, in or out of the kernel.

Fixes: 00bf25d693e7 ("y2038: use time32 syscall names on 32-bit")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: libc-alpha@sourceware.org
Cc: linux-api@vger.kernel.org
Cc: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Stepan Golosunov <stepan@golosunov.pp.ru>
Link: https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
Link: https://lkml.kernel.org/r/20190429131951.471701-1-arnd@arndb.de

---
 arch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 33687dddd86a..9092e0ffe4d3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
 	bool
 
 config 64BIT_TIME
-	def_bool ARCH_HAS_64BIT_TIME
+	def_bool y
 	help
 	  This should be selected by all architectures that need to support
 	  new system calls with a 64-bit time_t. This is relevant on all 32-bit

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-29  7:33 ` Thomas Gleixner
@ 2019-04-29 13:22   ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-29 13:22 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Joseph Myers, GNU C Library, Linux API,
	Linux Kernel Mailing List, Deepa Dinamani, Lukasz Majewski,
	Stepan Golosunov

On Mon, Apr 29, 2019 at 9:34 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Fri, 26 Apr 2019, Arnd Bergmann wrote:
>
> > As Stepan Golosunov points out, we made a small mistake in the
> > get_timespec64() function in the kernel. It was originally added under
> > the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
> > and 64-bit architectures, but when I did the conversion, I only turned
> > it on for 32-bit ones.
> >
> > The effect is that the get_timespec64() function never clears the upper
> > half of the tv_nsec field for 32-bit tasks in compat mode. Clearing this
> > is required for POSIX compliant behavior of functions that pass a
> > 'timespec' structure with a 64-bit tv_sec and a 32-bit tv_nsec, plus
> > uninitialized padding.
> >
> > The easiest fix for linux-5.1 is to just make the Kconfig symbol
> > unconditional, as it was originally intended. As a follow-up,
> > we should remove any #ifdef CONFIG_64BIT_TIME completely.
> >
> > Link: https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
> > Cc: Lukasz Majewski <lukma@denx.de>
> > Cc: Stepan Golosunov <stepan@golosunov.pp.ru>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Please apply this one as a bugfix for 5.1
>
> Can you provide a 'Fixes: ....' tag please?

Ok, resent both patches now. I also took the chance to add a clarification
for the point that Lukasz missed on the first submission.

       Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-26 14:25 [PATCH 1/2] y2038: make " Arnd Bergmann
  2019-04-26 22:46 ` Lukasz Majewski
@ 2019-04-29  7:33 ` Thomas Gleixner
  2019-04-29 13:22   ` Arnd Bergmann
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2019-04-29  7:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Joseph Myers, libc-alpha, linux-api, linux-kernel,
	Deepa Dinamani, Lukasz Majewski, Stepan Golosunov

On Fri, 26 Apr 2019, Arnd Bergmann wrote:

> As Stepan Golosunov points out, we made a small mistake in the
> get_timespec64() function in the kernel. It was originally added under
> the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
> and 64-bit architectures, but when I did the conversion, I only turned
> it on for 32-bit ones.
> 
> The effect is that the get_timespec64() function never clears the upper
> half of the tv_nsec field for 32-bit tasks in compat mode. Clearing this
> is required for POSIX compliant behavior of functions that pass a
> 'timespec' structure with a 64-bit tv_sec and a 32-bit tv_nsec, plus
> uninitialized padding.
> 
> The easiest fix for linux-5.1 is to just make the Kconfig symbol
> unconditional, as it was originally intended. As a follow-up,
> we should remove any #ifdef CONFIG_64BIT_TIME completely.
> 
> Link: https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Stepan Golosunov <stepan@golosunov.pp.ru>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Please apply this one as a bugfix for 5.1

Can you provide a 'Fixes: ....' tag please?

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-27 15:06     ` Lukasz Majewski
@ 2019-04-27 20:47       ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-27 20:47 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Stepan Golosunov, Thomas Gleixner, Joseph Myers, GNU C Library,
	Linux API, Linux Kernel Mailing List, Deepa Dinamani

On Sat, Apr 27, 2019 at 5:06 PM Lukasz Majewski <lukma@denx.de> wrote:
> > 27.04.2019 в 00:46:53 +0200 Lukasz Majewski написал:
> > (I am wondering whether such trucation is undefined behaviour in C
>
> According to [1] - Chapter 6.3.1.3 - Point 3 it is
> implementation-defined.

The kernel relies on the sane behavior for integer overflow in many
places already, and it is built with -fno-strict-overflow to also make
sure gcc doesn't optimize cases that would be undefined otherwise.

> > and
> > whether there should be sign-extension instead of zeroing-out for the
> > in_compat_syscall() case though.)
>
> What I've found is that "typically" the high order bits are discarded.
>
> However, it is still "implementation dependent".

I think the question was whether we should use

          kts.tv_nsec = (int)kts.tv_nsec;

instead of

          kts.tv_nsec &= 0xfffffffful;

Both have a clearly defined meaning in the C dialect we use in the
kernel, but differ in the upper 32 bits for negative input values.

I would say that using sign-extension indeed makes more sense
here, but we don't need to change it for linux-5.1, since none of the
callers of get_timespec64() care -- any negative 32-bit tv_nsec
value results in -EINVAL, including the utimensat() syscall that
has two special cases outside of the 0...999999999 range.

      Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-27  9:54   ` Stepan Golosunov
@ 2019-04-27 15:06     ` Lukasz Majewski
  2019-04-27 20:47       ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Lukasz Majewski @ 2019-04-27 15:06 UTC (permalink / raw)
  To: Stepan Golosunov
  Cc: Arnd Bergmann, Thomas Gleixner, Joseph Myers, libc-alpha,
	linux-api, linux-kernel, Deepa Dinamani

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

Hi Stepan,

> 27.04.2019 в 00:46:53 +0200 Lukasz Majewski написал:
> > Hi Arnd,
> >   
> > > As Stepan Golosunov points out, we made a small mistake in the
> > > get_timespec64() function in the kernel. It was originally added
> > > under the assumption that CONFIG_64BIT_TIME would get enabled on
> > > all 32-bit and 64-bit architectures, but when I did the
> > > conversion, I only turned it on for 32-bit ones.
> > > 
> > > The effect is that the get_timespec64() function never clears the
> > > upper half of the tv_nsec field for 32-bit tasks in compat mode.
> > > Clearing this is required for POSIX compliant behavior of
> > > functions that pass a 'timespec' structure with a 64-bit tv_sec
> > > and a 32-bit tv_nsec, plus uninitialized padding.  
> 
> > At least for my setup (32bit ARM with 64 bit time support) this
> > patch is not fixing anything.  
> 
> The patch is not supposed to fix anything on 32-bit architectures as
> in-kernel struct timespec64 has 32-bit tv_nsec there.  Thus truncation
> should happen automatically.  I also missed that fact when I was
> reading get_timespec64 code.

Yes. You are right. The tv_nsec is 32 bit.

> 
> (I am wondering whether such trucation is undefined behaviour in C

According to [1] - Chapter 6.3.1.3 - Point 3 it is
implementation-defined.

> and
> whether there should be sign-extension instead of zeroing-out for the
> in_compat_syscall() case though.)

What I've found is that "typically" the high order bits are discarded.

However, it is still "implementation dependent".

> 
> > > The easiest fix for linux-5.1 is to just make the Kconfig symbol
> > > unconditional, as it was originally intended. As a follow-up,
> > > we should remove any #ifdef CONFIG_64BIT_TIME completely.
> > > 
> > > Link:
> > > https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
> > > Cc: Lukasz Majewski <lukma@denx.de> Cc: Stepan Golosunov
> > > <stepan@golosunov.pp.ru> Signed-off-by: Arnd Bergmann
> > > <arnd@arndb.de> ---
> > > Please apply this one as a bugfix for 5.1
> > > ---
> > >  arch/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 33687dddd86a..9092e0ffe4d3 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
> > >  	bool
> > >  
> > >  config 64BIT_TIME
> > > -	def_bool ARCH_HAS_64BIT_TIME
> > > +	def_bool y
> > >  	help
> > >  	  This should be selected by all architectures that need
> > > to support new system calls with a 64-bit time_t. This is
> > > relevant on all 32-bit  

Note:

[1] - http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-26 22:46 ` Lukasz Majewski
@ 2019-04-27  9:54   ` Stepan Golosunov
  2019-04-27 15:06     ` Lukasz Majewski
  0 siblings, 1 reply; 10+ messages in thread
From: Stepan Golosunov @ 2019-04-27  9:54 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Arnd Bergmann, Thomas Gleixner, Joseph Myers, libc-alpha,
	linux-api, linux-kernel, Deepa Dinamani

27.04.2019 в 00:46:53 +0200 Lukasz Majewski написал:
> Hi Arnd,
> 
> > As Stepan Golosunov points out, we made a small mistake in the
> > get_timespec64() function in the kernel. It was originally added under
> > the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
> > and 64-bit architectures, but when I did the conversion, I only turned
> > it on for 32-bit ones.
> > 
> > The effect is that the get_timespec64() function never clears the
> > upper half of the tv_nsec field for 32-bit tasks in compat mode.
> > Clearing this is required for POSIX compliant behavior of functions
> > that pass a 'timespec' structure with a 64-bit tv_sec and a 32-bit
> > tv_nsec, plus uninitialized padding.

> At least for my setup (32bit ARM with 64 bit time support) this patch is
> not fixing anything.

The patch is not supposed to fix anything on 32-bit architectures as
in-kernel struct timespec64 has 32-bit tv_nsec there.  Thus truncation
should happen automatically.  I also missed that fact when I was
reading get_timespec64 code.

(I am wondering whether such trucation is undefined behaviour in C and
whether there should be sign-extension instead of zeroing-out for the
in_compat_syscall() case though.)

> > The easiest fix for linux-5.1 is to just make the Kconfig symbol
> > unconditional, as it was originally intended. As a follow-up,
> > we should remove any #ifdef CONFIG_64BIT_TIME completely.
> > 
> > Link:
> > https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
> > Cc: Lukasz Majewski <lukma@denx.de> Cc: Stepan Golosunov
> > <stepan@golosunov.pp.ru> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Please apply this one as a bugfix for 5.1
> > ---
> >  arch/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 33687dddd86a..9092e0ffe4d3 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
> >  	bool
> >  
> >  config 64BIT_TIME
> > -	def_bool ARCH_HAS_64BIT_TIME
> > +	def_bool y
> >  	help
> >  	  This should be selected by all architectures that need to
> > support new system calls with a 64-bit time_t. This is relevant on
> > all 32-bit

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
  2019-04-26 14:25 [PATCH 1/2] y2038: make " Arnd Bergmann
@ 2019-04-26 22:46 ` Lukasz Majewski
  2019-04-27  9:54   ` Stepan Golosunov
  2019-04-29  7:33 ` Thomas Gleixner
  1 sibling, 1 reply; 10+ messages in thread
From: Lukasz Majewski @ 2019-04-26 22:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Thomas Gleixner, Joseph Myers, libc-alpha, linux-api,
	linux-kernel, Deepa Dinamani, Stepan Golosunov

[-- Attachment #1: Type: text/plain, Size: 2890 bytes --]

Hi Arnd,

> As Stepan Golosunov points out, we made a small mistake in the
> get_timespec64() function in the kernel. It was originally added under
> the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
> and 64-bit architectures, but when I did the conversion, I only turned
> it on for 32-bit ones.
> 
> The effect is that the get_timespec64() function never clears the
> upper half of the tv_nsec field for 32-bit tasks in compat mode.
> Clearing this is required for POSIX compliant behavior of functions
> that pass a 'timespec' structure with a 64-bit tv_sec and a 32-bit
> tv_nsec, plus uninitialized padding.

On my 32 bit ARM setup (with CONFIG_COMPAT_32BIT_TIME=y) for
Y2038 test:

- I do use clock_settime64 [1] to set 64 bit tv_sec
  with 32 bit tv_nsec and 32 bit unnamed padding.

- I also may use clock_settime32 as a fallback (but it will not work
  beyond Y2038) if the 64 bit version is not available for any reason.

In the get_timespec64() the in_compat_syscall() returns 0 [2], so the
padding bits are not cleared.

This imposes the in glibc requirement to zero the padding before passing
it to the Linux kernel.

At least for my setup (32bit ARM with 64 bit time support) this patch is
not fixing anything.


Moreover, the described above problem doesn't matter on native 64 bit
systems as there both fields are 64 bit (no padding required).


Note:
[1] -
https://elixir.bootlin.com/linux/v5.1-rc6/source/arch/arm/tools/syscall.tbl#L421

[2] - include/linux/compat.h -> CONFIG_COMPAT is not defined - as a
result in_compat_syscall() returns 0.

> 
> The easiest fix for linux-5.1 is to just make the Kconfig symbol
> unconditional, as it was originally intended. As a follow-up,
> we should remove any #ifdef CONFIG_64BIT_TIME completely.
> 
> Link:
> https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
> Cc: Lukasz Majewski <lukma@denx.de> Cc: Stepan Golosunov
> <stepan@golosunov.pp.ru> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Please apply this one as a bugfix for 5.1
> ---
>  arch/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 33687dddd86a..9092e0ffe4d3 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
>  	bool
>  
>  config 64BIT_TIME
> -	def_bool ARCH_HAS_64BIT_TIME
> +	def_bool y
>  	help
>  	  This should be selected by all architectures that need to
> support new system calls with a 64-bit time_t. This is relevant on
> all 32-bit




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional
@ 2019-04-26 14:25 Arnd Bergmann
  2019-04-26 22:46 ` Lukasz Majewski
  2019-04-29  7:33 ` Thomas Gleixner
  0 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-26 14:25 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Joseph Myers, libc-alpha, linux-api, linux-kernel,
	Deepa Dinamani, Arnd Bergmann, Lukasz Majewski, Stepan Golosunov

As Stepan Golosunov points out, we made a small mistake in the
get_timespec64() function in the kernel. It was originally added under
the assumption that CONFIG_64BIT_TIME would get enabled on all 32-bit
and 64-bit architectures, but when I did the conversion, I only turned
it on for 32-bit ones.

The effect is that the get_timespec64() function never clears the upper
half of the tv_nsec field for 32-bit tasks in compat mode. Clearing this
is required for POSIX compliant behavior of functions that pass a
'timespec' structure with a 64-bit tv_sec and a 32-bit tv_nsec, plus
uninitialized padding.

The easiest fix for linux-5.1 is to just make the Kconfig symbol
unconditional, as it was originally intended. As a follow-up,
we should remove any #ifdef CONFIG_64BIT_TIME completely.

Link: https://lore.kernel.org/lkml/20190422090710.bmxdhhankurhafxq@sghpc.golosunov.pp.ru/
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Stepan Golosunov <stepan@golosunov.pp.ru>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Please apply this one as a bugfix for 5.1
---
 arch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 33687dddd86a..9092e0ffe4d3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -764,7 +764,7 @@ config COMPAT_OLD_SIGACTION
 	bool
 
 config 64BIT_TIME
-	def_bool ARCH_HAS_64BIT_TIME
+	def_bool y
 	help
 	  This should be selected by all architectures that need to support
 	  new system calls with a 64-bit time_t. This is relevant on all 32-bit
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-05-09  8:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 13:19 [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional Arnd Bergmann
2019-04-29 13:19 ` [PATCH 2/2] y2038: remove CONFIG_64BIT_TIME Arnd Bergmann
2019-05-09  8:52 ` [tip:timers/urgent] y2038: Make CONFIG_64BIT_TIME unconditional tip-bot for Arnd Bergmann
  -- strict thread matches above, loose matches on Subject: below --
2019-04-26 14:25 [PATCH 1/2] y2038: make " Arnd Bergmann
2019-04-26 22:46 ` Lukasz Majewski
2019-04-27  9:54   ` Stepan Golosunov
2019-04-27 15:06     ` Lukasz Majewski
2019-04-27 20:47       ` Arnd Bergmann
2019-04-29  7:33 ` Thomas Gleixner
2019-04-29 13:22   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).