Message ID | 20200413175117.170042-3-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | V2 [PATCH 2/2] Add a syscall test for [BZ #25810] | expand |
* 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 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.
* 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? And say that the test is in this formulation likely x32-specific, but that it is expected to work on other architectures as well.
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"); > And say that the test is in this formulation likely x32-specific, but > that it is expected to work on other architectures as well.
* 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. */
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..d07f03633b --- /dev/null +++ b/misc/tst-syscalls.c @@ -0,0 +1,146 @@ +/* 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/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <support/check.h> +#include <support/xunistd.h> + +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) +{ + 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) +{ + if (b.length) + return do_mmap (b.ptr, b.length); + return NULL; +} + +__attribute__ ((noclone, noinline)) +void +protect (struct Array b) +{ + if (b.length) + { + 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) +{ + 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) +{ + 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>