[BZ,#19371] Properly handle x32 syscall

Message ID CAMe9rOpAN+=AG9ucTUWUDMzW_dO-tizv2wX5qQA9Yf1iVWHU4g@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu Dec. 16, 2015, 9:31 p.m. UTC
  On Wed, Dec 16, 2015 at 8:38 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 12/16/2015 05:36 PM, H.J. Lu wrote:
>> On Wed, Dec 16, 2015 at 8:35 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 12/16/2015 04:01 PM, H.J. Lu wrote:
>>>> -extern long int syscall (long int __sysno, ...) __THROW;
>>>> +extern __syscall_slong_t syscall (long int __sysno, ...) __THROW;
>>>
>>> syscall is documented in the manual, so this change needs to be
>>> reflected there in some way.
>>
>> I will send a patch to Linux man pages after it is fixed in glibc.
>
> Sorry, I was talking about manual/startup.texi:
>
> manual/startup.texi:727:@deftypefun {long int} syscall (long int
> @var{sysno}, @dots{})
> manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> manual/startup.texi-729-
> manual/startup.texi-730-@code{syscall} performs a generic system call.
>

Here is the updated patch.  OK for master?

Thanks.
  

Comments

Joseph Myers Dec. 17, 2015, 12:53 a.m. UTC | #1
On Wed, 16 Dec 2015, H.J. Lu wrote:

> > Sorry, I was talking about manual/startup.texi:
> >
> > manual/startup.texi:727:@deftypefun {long int} syscall (long int
> > @var{sysno}, @dots{})
> > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > manual/startup.texi-729-
> > manual/startup.texi-730-@code{syscall} performs a generic system call.
> >
> 
> Here is the updated patch.  OK for master?

That can't possibly be right.  User documentation needs to be in terms of 
types exposed to the user (and documented as such), not anything starting 
__.
  
H.J. Lu Dec. 17, 2015, 2:02 a.m. UTC | #2
On Wed, Dec 16, 2015 at 4:53 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 16 Dec 2015, H.J. Lu wrote:
>
>> > Sorry, I was talking about manual/startup.texi:
>> >
>> > manual/startup.texi:727:@deftypefun {long int} syscall (long int
>> > @var{sysno}, @dots{})
>> > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
>> > manual/startup.texi-729-
>> > manual/startup.texi-730-@code{syscall} performs a generic system call.
>> >
>>
>> Here is the updated patch.  OK for master?
>
> That can't possibly be right.  User documentation needs to be in terms of
> types exposed to the user (and documented as such), not anything starting
> __.

It is lower priority for me since people should use lseek, time and
times instead of syscall ().
  
Rich Felker Dec. 21, 2015, 10:06 p.m. UTC | #3
On Thu, Dec 17, 2015 at 12:53:04AM +0000, Joseph Myers wrote:
> On Wed, 16 Dec 2015, H.J. Lu wrote:
> 
> > > Sorry, I was talking about manual/startup.texi:
> > >
> > > manual/startup.texi:727:@deftypefun {long int} syscall (long int
> > > @var{sysno}, @dots{})
> > > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > > manual/startup.texi-729-
> > > manual/startup.texi-730-@code{syscall} performs a generic system call.
> > >
> > 
> > Here is the updated patch.  OK for master?
> 
> That can't possibly be right.  User documentation needs to be in terms of 
> types exposed to the user (and documented as such), not anything starting 
> __.

I agree, and I'm very much opposed to all the hand-waving that's been
done with regards to breaking documented APIs by replacing long with
__syscall_slong_t (which is not even a type that's supposed to be
exposed to userspace) in the arguments, return types, and struct
members of public interfaces. This makes it impossible for portable
code using functions that may not be posix-standard, but which are
widely known and available, to support x32 without arch-specific
#ifdeffery nonsense.

Rich
  
Mike Frysinger Dec. 29, 2015, 10:53 p.m. UTC | #4
On 21 Dec 2015 17:06, Rich Felker wrote:
> On Thu, Dec 17, 2015 at 12:53:04AM +0000, Joseph Myers wrote:
> > On Wed, 16 Dec 2015, H.J. Lu wrote:
> > > > Sorry, I was talking about manual/startup.texi:
> > > >
> > > > manual/startup.texi:727:@deftypefun {long int} syscall (long int
> > > > @var{sysno}, @dots{})
> > > > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > > > manual/startup.texi-729-
> > > > manual/startup.texi-730-@code{syscall} performs a generic system call.
> > > 
> > > Here is the updated patch.  OK for master?
> > 
> > That can't possibly be right.  User documentation needs to be in terms of 
> > types exposed to the user (and documented as such), not anything starting 
> > __.
> 
> I agree, and I'm very much opposed to all the hand-waving that's been
> done with regards to breaking documented APIs by replacing long with
> __syscall_slong_t (which is not even a type that's supposed to be
> exposed to userspace) in the arguments, return types, and struct
> members of public interfaces. This makes it impossible for portable
> code using functions that may not be posix-standard, but which are
> widely known and available, to support x32 without arch-specific
> #ifdeffery nonsense.

syscall isn't portable.  it's not in POSIX, nor are the syscall numbers
used reliable across arches, nor are the func signatures.  claiming that
you don't need to worry about arch differences while also saying you can
safely use syscall directly is ridiculous.

glibc has created new types in the past.  creating a new one here doesn't
seem problematic to me.  we might also want to add a define to inttypes.h
so people have something to fallback on.  there's a number of types that
lack proper printf helpers already ...
-mike
  
Rich Felker Dec. 29, 2015, 11:21 p.m. UTC | #5
On Tue, Dec 29, 2015 at 05:53:38PM -0500, Mike Frysinger wrote:
> On 21 Dec 2015 17:06, Rich Felker wrote:
> > On Thu, Dec 17, 2015 at 12:53:04AM +0000, Joseph Myers wrote:
> > > On Wed, 16 Dec 2015, H.J. Lu wrote:
> > > > > Sorry, I was talking about manual/startup.texi:
> > > > >
> > > > > manual/startup.texi:727:@deftypefun {long int} syscall (long int
> > > > > @var{sysno}, @dots{})
> > > > > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > > > > manual/startup.texi-729-
> > > > > manual/startup.texi-730-@code{syscall} performs a generic system call.
> > > > 
> > > > Here is the updated patch.  OK for master?
> > > 
> > > That can't possibly be right.  User documentation needs to be in terms of 
> > > types exposed to the user (and documented as such), not anything starting 
> > > __.
> > 
> > I agree, and I'm very much opposed to all the hand-waving that's been
> > done with regards to breaking documented APIs by replacing long with
> > __syscall_slong_t (which is not even a type that's supposed to be
> > exposed to userspace) in the arguments, return types, and struct
> > members of public interfaces. This makes it impossible for portable
> > code using functions that may not be posix-standard, but which are
> > widely known and available, to support x32 without arch-specific
> > #ifdeffery nonsense.
> 
> syscall isn't portable.  it's not in POSIX, nor are the syscall numbers
> used reliable across arches, nor are the func signatures.  claiming that

I specifically meant portable across different Linux archs, not to
other different systems. But __syscall_slong_t has even crept into
places that do not require using syscall() but which have their own
documented (some of them even C11 and POSIX, e.g. timespec stuff)
interfaces. This is what I'm objecting to most strongly.

Rich
  
Mike Frysinger Dec. 29, 2015, 11:33 p.m. UTC | #6
On 29 Dec 2015 17:53, Mike Frysinger wrote:
> ... nor are the syscall numbers used reliable across arches, ...

to clarify, i obviously don't mean syscall(123).  i'm talking about diff
arches using diff names for the same func like __NR_getxpid/__NR_getpid
or __NR_chown32/__NR_chown or __NR_mmap/__NR_mmap2, among many others.
-mike
  
Mike Frysinger Dec. 29, 2015, 11:35 p.m. UTC | #7
On 29 Dec 2015 18:21, Rich Felker wrote:
> On Tue, Dec 29, 2015 at 05:53:38PM -0500, Mike Frysinger wrote:
> > On 21 Dec 2015 17:06, Rich Felker wrote:
> > > On Thu, Dec 17, 2015 at 12:53:04AM +0000, Joseph Myers wrote:
> > > > On Wed, 16 Dec 2015, H.J. Lu wrote:
> > > > > > Sorry, I was talking about manual/startup.texi:
> > > > > >
> > > > > > manual/startup.texi:727:@deftypefun {long int} syscall (long int
> > > > > > @var{sysno}, @dots{})
> > > > > > manual/startup.texi-728-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > > > > > manual/startup.texi-729-
> > > > > > manual/startup.texi-730-@code{syscall} performs a generic system call.
> > > > > 
> > > > > Here is the updated patch.  OK for master?
> > > > 
> > > > That can't possibly be right.  User documentation needs to be in terms of 
> > > > types exposed to the user (and documented as such), not anything starting 
> > > > __.
> > > 
> > > I agree, and I'm very much opposed to all the hand-waving that's been
> > > done with regards to breaking documented APIs by replacing long with
> > > __syscall_slong_t (which is not even a type that's supposed to be
> > > exposed to userspace) in the arguments, return types, and struct
> > > members of public interfaces. This makes it impossible for portable
> > > code using functions that may not be posix-standard, but which are
> > > widely known and available, to support x32 without arch-specific
> > > #ifdeffery nonsense.
> > 
> > syscall isn't portable.  it's not in POSIX, nor are the syscall numbers
> > used reliable across arches, nor are the func signatures.  claiming that
> 
> I specifically meant portable across different Linux archs, not to
> other different systems.

as i said, syscall isn't even portable at that level

> But __syscall_slong_t has even crept into
> places that do not require using syscall() but which have their own
> documented (some of them even C11 and POSIX, e.g. timespec stuff)
> interfaces. This is what I'm objecting to most strongly.

that's orthogonal to updating the syscall() API.
-mike
  

Patch

From d9a921b0ea019ba7768eb6cc81ece94e587f1660 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Dec 2015 06:50:42 -0800
Subject: [PATCH] Properly handle x32 syscall

X32 syscall() may return 64-bit integer as lseek, time and times.  Its
return type should be __syscall_slong_t instead of long int.  We need
to properly return 64-bit error value.

Before the patch:

Dump of assembler code for function syscall:
   0x000dab20 <+0>:	mov    %rdi,%rax
   0x000dab23 <+3>:	mov    %rsi,%rdi
   0x000dab26 <+6>:	mov    %rdx,%rsi
   0x000dab29 <+9>:	mov    %rcx,%rdx
   0x000dab2c <+12>:	mov    %r8,%r10
   0x000dab2f <+15>:	mov    %r9,%r8
   0x000dab32 <+18>:	mov    0x8(%rsp),%r9
   0x000dab37 <+23>:	syscall
   0x000dab39 <+25>:	cmp    $0xfffffffffffff001,%rax
   0x000dab3f <+31>:	jae    0xdab42 <syscall+34>
   0x000dab41 <+33>:	retq
   0x000dab42 <+34>:	mov    0x2b3367(%rip),%rcx        # 0x38deb0
   0x000dab49 <+41>:	neg    %eax
   0x000dab4b <+43>:	mov    %eax,%fs:(%rcx)
   0x000dab4e <+46>:	or     $0xffffffff,%eax
                        ^^^^^^^^^^^^^^^^^^ This is 32-bit error return.
   0x000dab51 <+49>:	retq
End of assembler dump.

After the patch:

Dump of assembler code for function syscall:
   0x000daaf0 <+0>:	mov    %rdi,%rax
   0x000daaf3 <+3>:	mov    %rsi,%rdi
   0x000daaf6 <+6>:	mov    %rdx,%rsi
   0x000daaf9 <+9>:	mov    %rcx,%rdx
   0x000daafc <+12>:	mov    %r8,%r10
   0x000daaff <+15>:	mov    %r9,%r8
   0x000dab02 <+18>:	mov    0x8(%rsp),%r9
   0x000dab07 <+23>:	syscall
   0x000dab09 <+25>:	cmp    $0xfffffffffffff001,%rax
   0x000dab0f <+31>:	jae    0xdab12 <syscall+34>
   0x000dab11 <+33>:	retq
   0x000dab12 <+34>:	mov    0x2b3397(%rip),%rcx        # 0x38deb0
   0x000dab19 <+41>:	neg    %eax
   0x000dab1b <+43>:	mov    %eax,%fs:(%rcx)
   0x000dab1e <+46>:	or     $0xffffffffffffffff,%rax
   0x000dab22 <+50>:	retq
End of assembler dump.

	[BZ #19371]
	* manual/startup.texi (syscall): Use __syscall_slong_t for
	return type.
	* posix/unistd.h (syscall): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/x32/syscall.S: New file.
---
 manual/startup.texi                          |  2 +-
 posix/unistd.h                               |  2 +-
 sysdeps/unix/sysv/linux/x86_64/x32/syscall.S | 33 ++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/syscall.S

diff --git a/manual/startup.texi b/manual/startup.texi
index 9a091a5..56d10fc 100644
--- a/manual/startup.texi
+++ b/manual/startup.texi
@@ -724,7 +724,7 @@  anyway.
 
 @comment unistd.h
 @comment ???
-@deftypefun {long int} syscall (long int @var{sysno}, @dots{})
+@deftypefun {__syscall_slong_t} syscall (long int @var{sysno}, @dots{})
 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 
 @code{syscall} performs a generic system call.
diff --git a/posix/unistd.h b/posix/unistd.h
index 1b52930..bb61ad7 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -1058,7 +1058,7 @@  extern void *sbrk (intptr_t __delta) __THROW;
 
    In Mach, all system calls take normal arguments and always return an
    error code (zero for success).  */
-extern long int syscall (long int __sysno, ...) __THROW;
+extern __syscall_slong_t syscall (long int __sysno, ...) __THROW;
 
 #endif	/* Use misc.  */
 
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/syscall.S b/sysdeps/unix/sysv/linux/x86_64/x32/syscall.S
new file mode 100644
index 0000000..cc5255f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/syscall.S
@@ -0,0 +1,33 @@ 
+/* The syscall system call.  Linux/x32 version.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sysdep.h>
+
+/* Return -1LL in a full 64 bits.  */
+#undef SYSCALL_ERROR_HANDLER
+#define SYSCALL_ERROR_HANDLER	\
+0:				\
+	SYSCALL_SET_ERRNO;	\
+	orq $-1, %rax;		\
+	ret;
+
+/* Always use our own error handler.  */
+#undef SYSCALL_ERROR_LABEL
+#define SYSCALL_ERROR_LABEL 0f
+
+#include <sysdeps/unix/sysv/linux/x86_64/syscall.S>
-- 
2.5.0