linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] parisc64: change __kernel_suseconds_t to match glibc
@ 2018-09-13 15:59 Arnd Bergmann
  2018-09-17 19:46 ` Helge Deller
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2018-09-13 15:59 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller
  Cc: y2038, Arnd Bergmann, Thomas Gleixner, linux-parisc, linux-kernel

There are only two 64-bit architecture ports that have a 32-bit
suseconds_t: sparc64 and parisc64. I've encountered a number of problems
with this, while trying to get a proper 64-bit time_t working on 32-bit
architectures. Having a 32-bit suseconds_t combined with a 64-bit time_t
means that we get extra padding in data structures that may leak kernel
stack data to user space, and it breaks all code that assumes that
timespec and timeval have the same layout.

While we can't change sparc64, it seems that glibc on parisc64 has always
set suseconds_t to 'long', and the current version would give incorrect
results for gettimeofday() and many other interfaces: timestamps passed
from user space into the kernel result in tv_usec being always zero
(the lower bits contain the intended value but are ignored) while data
passed from the kernel to user space contains either zeroes or random
data in tv_usec.

Based on that, it seems best to change the user API in the kernel in
an incompatible way to match what glibc expects.

Note that the distros I could find (gentoo and debian) all just
have 32-bit user space, which does not suffer from this problem.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/parisc/include/uapi/asm/posix_types.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/parisc/include/uapi/asm/posix_types.h b/arch/parisc/include/uapi/asm/posix_types.h
index 2785632c85e7..8dce56f5dcee 100644
--- a/arch/parisc/include/uapi/asm/posix_types.h
+++ b/arch/parisc/include/uapi/asm/posix_types.h
@@ -16,9 +16,6 @@ typedef unsigned short		__kernel_mode_t;
 typedef unsigned short		__kernel_ipc_pid_t;
 #define __kernel_ipc_pid_t __kernel_ipc_pid_t
 
-typedef int			__kernel_suseconds_t;
-#define __kernel_suseconds_t __kernel_suseconds_t
-
 typedef long long		__kernel_off64_t;
 typedef unsigned long long	__kernel_ino64_t;
 
-- 
2.18.0


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

* Re: [RFC] parisc64: change __kernel_suseconds_t to match glibc
  2018-09-13 15:59 [RFC] parisc64: change __kernel_suseconds_t to match glibc Arnd Bergmann
@ 2018-09-17 19:46 ` Helge Deller
  2018-09-25 10:53   ` Arnd Bergmann
  0 siblings, 1 reply; 4+ messages in thread
From: Helge Deller @ 2018-09-17 19:46 UTC (permalink / raw)
  To: Arnd Bergmann, James E.J. Bottomley
  Cc: y2038, Thomas Gleixner, linux-parisc, linux-kernel

Hi Arnd,

On 13.09.2018 17:59, Arnd Bergmann wrote:
> There are only two 64-bit architecture ports that have a 32-bit
> suseconds_t: sparc64 and parisc64. I've encountered a number of problems
> with this, while trying to get a proper 64-bit time_t working on 32-bit
> architectures. Having a 32-bit suseconds_t combined with a 64-bit time_t
> means that we get extra padding in data structures that may leak kernel
> stack data to user space, and it breaks all code that assumes that
> timespec and timeval have the same layout.
> 
> While we can't change sparc64, it seems that glibc on parisc64 has always
> set suseconds_t to 'long', and the current version would give incorrect
> results for gettimeofday() and many other interfaces: timestamps passed
> from user space into the kernel result in tv_usec being always zero
> (the lower bits contain the intended value but are ignored) while data
> passed from the kernel to user space contains either zeroes or random
> data in tv_usec.

Should this wrong behavior be visible with 32-bit userspace or
with 64-bit userspace (or both)?
I didn't noticed such wrong behavior yet.

Can you suggest some test programs to verify?
LTP seems to be correct.
 
> Based on that, it seems best to change the user API in the kernel in
> an incompatible way to match what glibc expects.

Basically that sounds the right way to go, but as said before,
I didn't see such issues.

> Note that the distros I could find (gentoo and debian) all just
> have 32-bit user space, which does not suffer from this problem.

That's correct.
Kernel can be 32- or 64-bit, but userspace is currentl 32-bit only.

So, breaking any 64-bit userspace is OK, since we don't have it yet.
Breaking 32-bit userspace needs some thoughts...

Helge


> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/parisc/include/uapi/asm/posix_types.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/parisc/include/uapi/asm/posix_types.h b/arch/parisc/include/uapi/asm/posix_types.h
> index 2785632c85e7..8dce56f5dcee 100644
> --- a/arch/parisc/include/uapi/asm/posix_types.h
> +++ b/arch/parisc/include/uapi/asm/posix_types.h
> @@ -16,9 +16,6 @@ typedef unsigned short		__kernel_mode_t;
>  typedef unsigned short		__kernel_ipc_pid_t;
>  #define __kernel_ipc_pid_t __kernel_ipc_pid_t
>  
> -typedef int			__kernel_suseconds_t;
> -#define __kernel_suseconds_t __kernel_suseconds_t
> -
>  typedef long long		__kernel_off64_t;
>  typedef unsigned long long	__kernel_ino64_t;
>  
> 


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

* Re: [RFC] parisc64: change __kernel_suseconds_t to match glibc
  2018-09-17 19:46 ` Helge Deller
@ 2018-09-25 10:53   ` Arnd Bergmann
  2018-09-25 17:53     ` Helge Deller
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2018-09-25 10:53 UTC (permalink / raw)
  To: Helge Deller
  Cc: James E.J. Bottomley, y2038 Mailman List, Thomas Gleixner,
	Parisc List, Linux Kernel Mailing List

On Mon, Sep 17, 2018 at 9:46 PM Helge Deller <deller@gmx.de> wrote:
> On 13.09.2018 17:59, Arnd Bergmann wrote:
> > There are only two 64-bit architecture ports that have a 32-bit
> > suseconds_t: sparc64 and parisc64. I've encountered a number of problems
> > with this, while trying to get a proper 64-bit time_t working on 32-bit
> > architectures. Having a 32-bit suseconds_t combined with a 64-bit time_t
> > means that we get extra padding in data structures that may leak kernel
> > stack data to user space, and it breaks all code that assumes that
> > timespec and timeval have the same layout.
> >
> > While we can't change sparc64, it seems that glibc on parisc64 has always
> > set suseconds_t to 'long', and the current version would give incorrect
> > results for gettimeofday() and many other interfaces: timestamps passed
> > from user space into the kernel result in tv_usec being always zero
> > (the lower bits contain the intended value but are ignored) while data
> > passed from the kernel to user space contains either zeroes or random
> > data in tv_usec.

[back from traveling now, sorry for the delay in replying]

> Should this wrong behavior be visible with 32-bit userspace or
> with 64-bit userspace (or both)?
> I didn't noticed such wrong behavior yet.

Only 64-bit user space.

> Can you suggest some test programs to verify?
> LTP seems to be correct.

A simple 64-bit gettimeofday() should report incorrect
nanoseconds using the upstream glibc implementation.

> > Based on that, it seems best to change the user API in the kernel in
> > an incompatible way to match what glibc expects.
>
> Basically that sounds the right way to go, but as said before,
> I didn't see such issues.
>
> > Note that the distros I could find (gentoo and debian) all just
> > have 32-bit user space, which does not suffer from this problem.
>
> That's correct.
> Kernel can be 32- or 64-bit, but userspace is currentl 32-bit only.
>
> So, breaking any 64-bit userspace is OK, since we don't have it yet.
> Breaking 32-bit userspace needs some thoughts...

Ok.

     Arnd

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

* Re: [RFC] parisc64: change __kernel_suseconds_t to match glibc
  2018-09-25 10:53   ` Arnd Bergmann
@ 2018-09-25 17:53     ` Helge Deller
  0 siblings, 0 replies; 4+ messages in thread
From: Helge Deller @ 2018-09-25 17:53 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: James E.J. Bottomley, y2038 Mailman List, Thomas Gleixner,
	Parisc List, Linux Kernel Mailing List

On 25.09.2018 12:53, Arnd Bergmann wrote:
> On Mon, Sep 17, 2018 at 9:46 PM Helge Deller <deller@gmx.de> wrote:
>> On 13.09.2018 17:59, Arnd Bergmann wrote:
>>> There are only two 64-bit architecture ports that have a 32-bit
>>> suseconds_t: sparc64 and parisc64. I've encountered a number of problems
>>> with this, while trying to get a proper 64-bit time_t working on 32-bit
>>> architectures. Having a 32-bit suseconds_t combined with a 64-bit time_t
>>> means that we get extra padding in data structures that may leak kernel
>>> stack data to user space, and it breaks all code that assumes that
>>> timespec and timeval have the same layout.
>>>
>>> While we can't change sparc64, it seems that glibc on parisc64 has always
>>> set suseconds_t to 'long', and the current version would give incorrect
>>> results for gettimeofday() and many other interfaces: timestamps passed
>>> from user space into the kernel result in tv_usec being always zero
>>> (the lower bits contain the intended value but are ignored) while data
>>> passed from the kernel to user space contains either zeroes or random
>>> data in tv_usec.
> 
> [back from traveling now, sorry for the delay in replying]
> 
>> Should this wrong behavior be visible with 32-bit userspace or
>> with 64-bit userspace (or both)?
>> I didn't noticed such wrong behavior yet.
> 
> Only 64-bit user space.
> 
...
> A simple 64-bit gettimeofday() should report incorrect
> nanoseconds using the upstream glibc implementation.

Yes, you are right.
Since we don't have any 64-bit userspace yet, it's safe to fix it now as you suggested.
I've added your patch as is to my for-next tree and tagged it for stable-tree.

Thanks for catching this!

Helge

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

end of thread, other threads:[~2018-09-25 17:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 15:59 [RFC] parisc64: change __kernel_suseconds_t to match glibc Arnd Bergmann
2018-09-17 19:46 ` Helge Deller
2018-09-25 10:53   ` Arnd Bergmann
2018-09-25 17:53     ` Helge Deller

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).