V2 [PATCH 2/2] Add a syscall test for [BZ #25810]
Commit Message
Add a test to pass 64-bit long arguments to syscall with undefined upper
32 bits on ILP32 systems.
Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
misc/Makefile | 2 +-
misc/tst-syscalls.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 misc/tst-syscalls.c
Comments
* 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. */
@@ -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))
new file mode 100644
@@ -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>