Re: [PATCH 1/2] y2038: make CONFIG_64BIT_TIME unconditional

[Lukasz Majewski]

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@xxxxxxxxxxxxxxxxxxxxx/
> > > Cc: Lukasz Majewski <lukma@xxxxxxx> Cc: Stepan Golosunov
> > > <stepan@xxxxxxxxxxxxxxx> Signed-off-by: Arnd Bergmann
> > > <arnd@xxxxxxxx> ---
> > > 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@xxxxxxx

Attachment: pgpqgmSARBdhp.pgp
Description: OpenPGP digital signature