diff mbox series

V3 [PATCH 2/2] Add a syscall test for [BZ #25810]

Message ID 20200422155202.GA342519@gmail.com
State New
Headers show
Series V3 [PATCH 2/2] Add a syscall test for [BZ #25810] | expand

Commit Message

H.J. Lu April 22, 2020, 3:52 p.m. UTC
On Wed, Apr 22, 2020 at 03:46:28PM +0200, Florian Weimer wrote:
> * H. J. Lu:
> 
> > On Wed, Apr 22, 2020 at 5:47 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >> >>
> >> >> * H. J. Lu via Libc-alpha:
> >> >>
> >> >> > Add a test to pass 64-bit long arguments to syscall with undefined upper
> >> >> > 32 bits on ILP32 systems.
> >> >>
> >> >> What does this test, exactly?  How does it ensure that the upper bits
> >> >> are indeed non-zero, to exercise the zero-extension case?
> >> >
> >> > On x32,
> >> >
> >> > struct Array
> >> > {
> >> >   size_t length;
> >> >   void *ptr;
> >> > };
> >> >
> >> > can be passed in a single 64-bit integer register.  When a 32-bit
> >> > integer is passed in
> >> > a 64-bit integer, its upper 32 bits can contain undefined value:
> >> >
> >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541
> >> >
> >> > This testcase arranges syscalls in such a way that the upper 32 bits
> >> > of 64 big integer
> >> > register, which is used to pass unsigned long to kernel, contains the
> >> > "ptr" value passed in
> >> > function arguments.   If the upper 32 bits aren't cleared, syscall
> >> > will fail since long in kernel
> >> > is 64 bits, not 32 bits.
> >>
> >> Would you please add this as a comment to the patch, and one-line
> >> comments where the clobbers happen?
> >
> > I will add
> >
> > +/* On x32, this can be passed in a single 64-bit integer register.  */
> >  struct Array
> >  {
> >    size_t length;
> > @@ -50,6 +51,9 @@ __attribute__ ((noclone, noinline))
> >  void
> >  deallocate (struct Array b)
> >  {
> > +  /* On x32, the 64-bit integer register containing `b' may be copied
> > +     to another 64-bit integer register to pass the second argument to
> > +     munmap.  */
> >    if (b.length && munmap (b.ptr, b.length))
> >      {
> >        printf ("munmap error: %m\n");
> 
> Looks good.
> 
> Please also add something like this at the top of the file, after the
> copyright header.
> 
> /* This test verifies that the x32 system call handling zero-extends
>    unsigned 32-bit arguments to the 64-bit argument registers for
>    system calls (bug 25810).  The bug is specific to x32, but the test
>    should pass on all architectures.  */

Here is the updated patch.  OK for master?

Thanks.

H.J.
---
Add a test to pass 64-bit long arguments to syscall with undefined upper
32 bits on x32.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
 misc/Makefile       |   2 +-
 misc/tst-syscalls.c | 167 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 misc/tst-syscalls.c

Comments

Florian Weimer April 22, 2020, 3:55 p.m. UTC | #1
* H. J. Lu:

>> Please also add something like this at the top of the file, after the
>> copyright header.
>> 
>> /* This test verifies that the x32 system call handling zero-extends
>>    unsigned 32-bit arguments to the 64-bit argument registers for
>>    system calls (bug 25810).  The bug is specific to x32, but the test
>>    should pass on all architectures.  */

I realize now that the phrasing is a bit awkward.  But at least the
bug number is there.

> Here is the updated patch.  OK for master?

Yes, fine with me.  Thanks for your patience.
diff mbox series

Patch

diff --git a/misc/Makefile b/misc/Makefile
index b8fed5783d..67c5237f97 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -87,7 +87,7 @@  tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \
 	 tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \
 	 tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \
 	 tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \
-	 tst-mntent-autofs
+	 tst-mntent-autofs tst-syscalls
 
 # Tests which need libdl.
 ifeq (yes,$(build-shared))
diff --git a/misc/tst-syscalls.c b/misc/tst-syscalls.c
new file mode 100644
index 0000000000..cfcd382320
--- /dev/null
+++ b/misc/tst-syscalls.c
@@ -0,0 +1,167 @@ 
+/* Test for syscall interfaces.
+   Copyright (C) 2020 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
+   <https://www.gnu.org/licenses/>.  */
+
+/* This test verifies that the x32 system call handling zero-extends
+   unsigned 32-bit arguments to the 64-bit argument registers for
+   system calls (bug 25810).  The bug is specific to x32, but the test
+   should pass on all architectures.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+/* On x32, this can be passed in a single 64-bit integer register.  */
+struct Array
+{
+  size_t length;
+  void *ptr;
+};
+
+static int error_count;
+
+__attribute__ ((noclone, noinline))
+struct Array
+allocate (size_t bytes)
+{
+  if (!bytes)
+    return __extension__ (struct Array) {0, 0};
+
+  void *p = mmap (0x0, bytes, PROT_READ | PROT_WRITE,
+		  MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (p == MAP_FAILED)
+    return __extension__ (struct Array) {0, 0};
+
+  return __extension__ (struct Array) {bytes, p};
+}
+
+__attribute__ ((noclone, noinline))
+void
+deallocate (struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     munmap.  */
+  if (b.length && munmap (b.ptr, b.length))
+    {
+      printf ("munmap error: %m\n");
+      error_count++;
+    }
+}
+
+__attribute__ ((noclone, noinline))
+void *
+do_mmap (void *addr, size_t length)
+{
+  return mmap (addr, length, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+}
+
+__attribute__ ((noclone, noinline))
+void *
+reallocate (struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     do_mmap.  */
+  if (b.length)
+    return do_mmap (b.ptr, b.length);
+  return NULL;
+}
+
+__attribute__ ((noclone, noinline))
+void
+protect (struct Array b)
+{
+  if (b.length)
+    {
+      /* On x32, the 64-bit integer register containing `b' may be copied
+	 to another 64-bit integer register to pass the second argument
+	 to mprotect.  */
+      if (mprotect (b.ptr, b.length,
+		    PROT_READ | PROT_WRITE | PROT_EXEC))
+	{
+	  printf ("mprotect error: %m\n");
+	  error_count++;
+	}
+    }
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_read (int fd, void *ptr, struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     read.  */
+  if (b.length)
+    return read (fd, ptr, b.length);
+  return 0;
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_write (int fd, void *ptr, struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     write.  */
+  if (b.length)
+    return write (fd, ptr, b.length);
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  struct Array array;
+
+  array = allocate (1);
+  protect (array);
+  deallocate (array);
+  void *p = reallocate (array);
+  if (p == MAP_FAILED)
+    {
+      printf ("mmap error: %m\n");
+      error_count++;
+    }
+  array.ptr = p;
+  protect (array);
+  deallocate (array);
+
+  int fd = xopen ("/dev/null", O_RDWR, 0);
+  char buf[2];
+  array.ptr = buf;
+  if (do_read (fd, array.ptr, array) == -1)
+    {
+      printf ("read error: %m\n");
+      error_count++;
+    }
+  if (do_write (fd, array.ptr, array) == -1)
+    {
+      printf ("write error: %m\n");
+      error_count++;
+    }
+  xclose (fd);
+
+  return error_count ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>