[v2] Test errno setup

Message ID 1487411274-21877-1-git-send-email-ynorov@caviumnetworks.com
State New
Headers show

Commit Message

Yury Norov Feb. 18, 2017, 9:47 a.m.
This patch adds tests for POSIX and Linux specific syscalls
that implemented with syscall templates machinery. The reason
of tests is to receive the expected error code and test if
it's handled properly by glibc.

v1: https://sourceware.org/ml/libc-alpha/2017-02/msg00276.html
v2: added FSF copyright headers;
    support/test-driver.c is used instead of test-skeleton.c;
    fixed misspeling in description;
    'FAIL' prefix added in test wrapper macro fail paths.

Yury Norov <ynorov@caviumnetworks.com>
Zack Weinberg <zackw@panix.com>

        * posix/Makefile: Enable errno test for POSIX syscalls.
        * posix/test-errno.c: New file.
        * sysdeps/unix/sysv/linux/Makefile: Enable errno test for Linux syscalls.
        * sysdeps/unix/sysv/linux/test-errno.c: New file.

---
 posix/Makefile                       |   2 +-
 posix/test-errno.c                   | 153 +++++++++++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/Makefile     |   2 +-
 sysdeps/unix/sysv/linux/test-errno.c | 142 ++++++++++++++++++++++++++++++++
 4 files changed, 297 insertions(+), 2 deletions(-)
 create mode 100644 posix/test-errno.c
 create mode 100644 sysdeps/unix/sysv/linux/test-errno.c

-- 
2.7.4

Comments

Yury Norov Feb. 27, 2017, 2:32 p.m. | #1
On Sat, Feb 18, 2017 at 03:17:54PM +0530, Yury Norov wrote:
> This patch adds tests for POSIX and Linux specific syscalls

> that implemented with syscall templates machinery. The reason

> of tests is to receive the expected error code and test if

> it's handled properly by glibc.

> 

> v1: https://sourceware.org/ml/libc-alpha/2017-02/msg00276.html

> v2: added FSF copyright headers;

>     support/test-driver.c is used instead of test-skeleton.c;

>     fixed misspeling in description;

>     'FAIL' prefix added in test wrapper macro fail paths.

> 

> Yury Norov <ynorov@caviumnetworks.com>

> Zack Weinberg <zackw@panix.com>


Ping?
Wainer dos Santos Moschetta March 6, 2017, 8:49 p.m. | #2
LGTM.


On 02/18/2017 07:47 AM, Yury Norov wrote:
> This patch adds tests for POSIX and Linux specific syscalls

> that implemented with syscall templates machinery. The reason

> of tests is to receive the expected error code and test if

> it's handled properly by glibc.

>

> v1: https://sourceware.org/ml/libc-alpha/2017-02/msg00276.html

> v2: added FSF copyright headers;

>     support/test-driver.c is used instead of test-skeleton.c;

>     fixed misspeling in description;

>     'FAIL' prefix added in test wrapper macro fail paths.

>

> Yury Norov <ynorov@caviumnetworks.com>

> Zack Weinberg <zackw@panix.com>

>

>         * posix/Makefile: Enable errno test for POSIX syscalls.

>         * posix/test-errno.c: New file.

>         * sysdeps/unix/sysv/linux/Makefile: Enable errno test for Linux syscalls.

>         * sysdeps/unix/sysv/linux/test-errno.c: New file.

>

> ---

>  posix/Makefile                       |   2 +-

>  posix/test-errno.c                   | 153 +++++++++++++++++++++++++++++++++++

>  sysdeps/unix/sysv/linux/Makefile     |   2 +-

>  sysdeps/unix/sysv/linux/test-errno.c | 142 ++++++++++++++++++++++++++++++++

>  4 files changed, 297 insertions(+), 2 deletions(-)

>  create mode 100644 posix/test-errno.c

>  create mode 100644 sysdeps/unix/sysv/linux/test-errno.c

>

> diff --git a/posix/Makefile b/posix/Makefile

> index 8f23d64..ae17646 100644

> --- a/posix/Makefile

> +++ b/posix/Makefile

> @@ -62,7 +62,7 @@ routines :=								      \

>  	get_child_max sched_cpucount sched_cpualloc sched_cpufree

>

>  aux		:= init-posix environ

> -tests		:= tstgetopt testfnm runtests runptests	     \

> +tests		:= test-errno tstgetopt testfnm runtests runptests \

>  		   tst-preadwrite tst-preadwrite64 test-vfork regexbug1 \

>  		   tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \

>  		   tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \

> diff --git a/posix/test-errno.c b/posix/test-errno.c

> new file mode 100644

> index 0000000..d484d02

> --- /dev/null

> +++ b/posix/test-errno.c

> @@ -0,0 +1,153 @@

> +/* Test that failing system calls do set errno to the correct value.

> +

> +   Copyright (C) 2017 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 <errno.h>

> +#include <limits.h>

> +#include <grp.h>

> +#include <fcntl.h>

> +#include <stdio.h>

> +#include <string.h>

> +#include <sys/ioctl.h>

> +#include <sys/socket.h>

> +#include <sys/stat.h>

> +#include <sys/time.h>

> +#include <sys/resource.h>

> +#include <sys/types.h>

> +#include <sys/statfs.h>

> +#include <sys/mman.h>

> +#include <unistd.h>

> +#include <netinet/in.h>

> +

> +/* This is not an exhaustive test: only system calls that can be

> +   persuaded to fail with a consistent error code and no side effects

> +   are included.  Usually these are failures due to invalid arguments,

> +   with errno code EBADF or EINVAL.  The order of argument checks is

> +   unspecified, so we must take care to provide arguments that only

> +   allow _one_ failure mode.

> +

> +   Note that all system calls that can fail with EFAULT are permitted

> +   to deliver a SIGSEGV signal instead, so we avoid supplying invalid

> +   pointers in general, and we do not attempt to test system calls

> +   that can only fail with EFAULT (e.g. gettimeofday, gethostname).

> +

> +   Also note that root-only system calls (e.g. acct, reboot) may, when

> +   the test is run as an unprivileged user, fail due to insufficient

> +   privileges before bothering to do argument checks, so those are not

> +   tested either.

> +

> +   Also, system calls that take enum or a set of flags as argument is

> +   not tested if POSIX doesn't specify exact binary values for all

> +   flags, and so any value passed to flags may become valid.

> +

> +   Some tests assume "/bin/sh" names a file that exists and is not a

> +   directory.  */

> +

> +#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\

> +  (__extension__ ({						\

> +    errno = 0xdead;						\

> +    rtype ret = syscall (__VA_ARGS__);				\

> +    int err = errno;						\

> +    int fail;							\

> +    if (ret == (rtype) -1 && err == experr)			\

> +      fail = 0;							\

> +    else							\

> +      {								\

> +        fail = 1;						\

> +        if (ret != (rtype) -1)					\

> +          printf ("FAIL: " #syscall ": didn't fail as expected"	\

> +               " (return "prtype")\n", ret);			\

> +        else if (err == 0xdead)					\

> +          puts("FAIL: " #syscall ": didn't update errno\n");	\

> +        else if (err != experr)					\

> +          printf ("FAIL: " #syscall				\

> +               ": errno is: %d (%s) expected: %d (%s)\n",	\

> +               err, strerror (err), experr, strerror (experr));	\

> +      }								\

> +    fail;							\

> +  }))

> +

> +#define test_wrp(experr, syscall, ...)				\

> +  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)

> +

> +static int

> +do_test (void)

> +{

> +  size_t pagesize = sysconf (_SC_PAGESIZE);

> +  struct statfs sfs;

> +  struct sockaddr sa;

> +  socklen_t sl;

> +  char buf[1];

> +  struct iovec iov[1] = { { buf, 1 } };

> +  struct sockaddr_in sin;

> +  sin.sin_family = AF_INET;

> +  sin.sin_port = htons (1026);

> +  sin.sin_addr.s_addr = htonl (INADDR_LOOPBACK);

> +  struct msghdr msg;

> +  memset(&msg, 0, sizeof msg);

> +  msg.msg_iov = iov;

> +  msg.msg_iovlen = 1;

> +

> +  int fails = 0;

> +  fails |= test_wrp (EBADF, accept, -1, &sa, &sl);

> +  fails |= test_wrp (EINVAL, access, "/", -1);

> +  fails |= test_wrp (EBADF, bind, -1, (struct sockaddr *)&sin, sizeof sin);

> +  fails |= test_wrp (ENOTDIR, chdir, "/bin/sh");

> +  fails |= test_wrp (EBADF, close, -1);

> +  fails |= test_wrp (EBADF, connect, -1, (struct sockaddr *)&sin, sizeof sin);

> +  fails |= test_wrp (EBADF, dup, -1);

> +  fails |= test_wrp (EBADF, dup2, -1, -1);

> +  fails |= test_wrp (EBADF, fchdir, -1);

> +  fails |= test_wrp (EBADF, fchmod, -1, 0);

> +  fails |= test_wrp (EBADF, fcntl, -1, 0);

> +  fails |= test_wrp (EBADF, fstatfs, -1, &sfs);

> +  fails |= test_wrp (EBADF, fsync, -1);

> +  fails |= test_wrp (EBADF, ftruncate, -1, 0);

> +  fails |= test_wrp (EINVAL, getgroups, -1, 0);

> +  fails |= test_wrp (EBADF, getpeername, -1, &sa, &sl);

> +  fails |= test_wrp (EBADF, getsockname, -1, &sa, &sl);

> +  fails |= test_wrp (EBADF, getsockopt, -1, 0, 0, buf, &sl);

> +  fails |= test_wrp (EBADF, ioctl, -1, TIOCNOTTY);

> +  fails |= test_wrp (EBADF, listen, -1, 1);

> +  fails |= test_wrp (EBADF, lseek, -1, 0, 0);

> +  fails |= test_wrp (EINVAL, madvise, (void *) -1, -1, 0);

> +  fails |= test_wrp_rv (void *, "%p", EBADF,

> +                        mmap, 0, pagesize, PROT_READ, MAP_PRIVATE, -1, 0);

> +  fails |= test_wrp (EINVAL, mprotect, (void *) -1, pagesize, -1);

> +  fails |= test_wrp (EINVAL, msync, (void *) -1, pagesize, -1);

> +  fails |= test_wrp (EINVAL, munmap, (void *) -1, 0);

> +  fails |= test_wrp (EINVAL, open, "/bin/sh", -1, 0);

> +  fails |= test_wrp (EBADF, read, -1, buf, 1);

> +  fails |= test_wrp (EINVAL, readlink, "/", buf, -1);

> +  fails |= test_wrp (EBADF, readv, -1, iov, 1);

> +  fails |= test_wrp (EBADF, recv, -1, buf, 1, 0);

> +  fails |= test_wrp (EBADF, recvfrom, -1, buf, 1, 0, &sa, &sl);

> +  fails |= test_wrp (EBADF, recvmsg, -1, &msg, 0);

> +  fails |= test_wrp (EINVAL, select, -1, 0, 0, 0, 0);

> +  fails |= test_wrp (EBADF, send, -1, buf, 1, 0);

> +  fails |= test_wrp (EBADF, sendmsg, -1, &msg, 0);

> +  fails |= test_wrp (EBADF, sendto, -1, buf, 1, 0, &sa, sl);

> +  fails |= test_wrp (EBADF, setsockopt, -1, 0, 0, buf, sl);

> +  fails |= test_wrp (EBADF, shutdown, -1, SHUT_RD);

> +  fails |= test_wrp (EBADF, write, -1, "Hello", sizeof ("Hello") );

> +  fails |= test_wrp (EBADF, writev, -1, iov, 1 );

> +

> +  return fails;

> +}

> +

> +#include "support/test-driver.c"

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile

> index b3d6866..3c0c89f 100644

> --- a/sysdeps/unix/sysv/linux/Makefile

> +++ b/sysdeps/unix/sysv/linux/Makefile

> @@ -43,7 +43,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \

>  		  bits/mman-linux.h

>

>  tests += tst-clone tst-clone2 tst-fanotify tst-personality tst-quota \

> -	 tst-sync_file_range

> +	 tst-sync_file_range test-errno

>

>  # Generate the list of SYS_* macros for the system calls (__NR_* macros).

>

> diff --git a/sysdeps/unix/sysv/linux/test-errno.c b/sysdeps/unix/sysv/linux/test-errno.c

> new file mode 100644

> index 0000000..b768ff0

> --- /dev/null

> +++ b/sysdeps/unix/sysv/linux/test-errno.c

> @@ -0,0 +1,142 @@

> +/* Test that failing system calls do set errno to the correct value.

> +

> +   Copyright (C) 2017 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 <errno.h>

> +#include <fcntl.h>

> +#include <mqueue.h>

> +#include <sched.h>

> +#include <signal.h>

> +#include <stdio.h>

> +#include <string.h>

> +#include <time.h>

> +#include <unistd.h>

> +#include <sys/epoll.h>

> +#include <sys/eventfd.h>

> +#include <sys/file.h>

> +#include <sys/fsuid.h>

> +#include <sys/inotify.h>

> +#include <sys/mman.h>

> +#include <sys/poll.h>

> +#include <sys/quota.h>

> +#include <sys/resource.h>

> +#include <sys/select.h>

> +#include <sys/sendfile.h>

> +#include <sys/swap.h>

> +#include <sys/time.h>

> +#include <sys/types.h>

> +#include <sys/wait.h>

> +

> +/* This is not an exhaustive test: only system calls that can be

> +   persuaded to fail with a consistent error code and no side effects

> +   are included.  Usually these are failures due to invalid arguments,

> +   with errno code EBADF or EINVAL.  The order of argument checks is

> +   unspecified, so we must take care to provide arguments that only

> +   allow _one_ failure mode.

> +

> +   Note that all system calls that can fail with EFAULT are permitted

> +   to deliver a SIGSEGV signal instead, so we avoid supplying invalid

> +   pointers in general, and we do not attempt to test system calls

> +   that can only fail with EFAULT (e.g. gettimeofday, gethostname).

> +

> +   Also note that root-only system calls (e.g. acct, reboot) may, when

> +   the test is run as an unprivileged user, fail due to insufficient

> +   privileges before bothering to do argument checks, so those are not

> +   tested either.

> +

> +   Also, system calls that take enum or a set of flags as argument is

> +   not tested if POSIX doesn't specify exact binary values for all

> +   flags, and so any value passed to flags may become valid.

> +

> +   Some tests assume "/bin/sh" names a file that exists and is not a

> +   directory.  */

> +

> +#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\

> +  (__extension__ ({						\

> +    errno = 0xdead;						\

> +    rtype ret = syscall (__VA_ARGS__);				\

> +    int err = errno;						\

> +    int fail;							\

> +    if (ret == (rtype) -1 && err == experr)			\

> +      fail = 0;							\

> +    else							\

> +      {								\

> +        fail = 1;						\

> +        if (ret != (rtype) -1)					\

> +          printf ("FAIL: " #syscall ": didn't fail as expected"	\

> +               " (return "prtype")\n", ret);			\

> +        else if (err == 0xdead)					\

> +          puts("FAIL: " #syscall ": didn't update errno\n");	\

> +        else if (err != experr)					\

> +          printf ("FAIL: " #syscall				\

> +               ": errno is: %d (%s) expected: %d (%s)\n",	\

> +               err, strerror (err), experr, strerror (experr));	\

> +      }								\

> +    fail;							\

> +  }))

> +

> +#define test_wrp(experr, syscall, ...)				\

> +  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)

> +

> +static int

> +do_test (void)

> +{

> +  fd_set rs, ws, es;

> +  int status;

> +  off_t off;

> +  stack_t ss;

> +  struct dqblk dqblk;

> +  struct epoll_event epoll_event;

> +  struct pollfd pollfd;

> +  struct sched_param sch_param;

> +  struct timespec ts;

> +  struct timeval tv;

> +  unsigned char vec[16];

> +  ss.ss_flags = ~SS_DISABLE;

> +  ts.tv_sec = -1;

> +

> +  int fails = 0;

> +  fails |= test_wrp (EINVAL, epoll_create, -1);

> +  fails |= test_wrp (EINVAL, epoll_create1, EPOLL_CLOEXEC + 1);

> +  fails |= test_wrp (EBADF, epoll_ctl, -1, EPOLL_CTL_ADD, 0, &epoll_event);

> +  fails |= test_wrp (EBADF, epoll_wait, -1, &epoll_event, 1, 1);

> +  fails |= test_wrp (EBADF, fdatasync, -1);

> +  fails |= test_wrp (EBADF, flock, -1, LOCK_SH);

> +  fails |= test_wrp (ESRCH, getpgid, -1);

> +  fails |= test_wrp (EINVAL, inotify_add_watch, -1, "/", 0);

> +  fails |= test_wrp (EINVAL, mincore, (void *) -1, 0, vec);

> +  fails |= test_wrp (EINVAL, mlock, (void *) -1, 1); // different errors

> +  fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);

> +  fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);

> +  fails |= test_wrp (ENODEV, quotactl, Q_GETINFO, NULL, -1, (caddr_t) &dqblk);

> +  fails |= test_wrp (EINVAL, sched_getparam, -1, &sch_param);

> +  fails |= test_wrp (EINVAL, sched_getscheduler, -1);

> +  fails |= test_wrp (EINVAL, sched_get_priority_max, -1);

> +  fails |= test_wrp (EINVAL, sched_get_priority_min, -1);

> +  fails |= test_wrp (EINVAL, sched_rr_get_interval, -1, &ts);

> +  fails |= test_wrp (EINVAL, sched_setparam, -1, &sch_param);

> +  fails |= test_wrp (EINVAL, sched_setscheduler, -1, 0, &sch_param);

> +  fails |= test_wrp (EINVAL, select, -1, &rs, &ws, &es, &tv);

> +  fails |= test_wrp (EBADF, sendfile, -1, -1, &off, 0);

> +  fails |= test_wrp (EINVAL, sigaltstack, &ss, NULL);

> +  fails |= test_wrp (ECHILD, wait4, -1, &status, 0, NULL);

> +

> +  return fails;

> +}

> +

> +#include "support/test-driver.c"
Yury Norov March 7, 2017, 2:42 a.m. | #3
On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta wrote:
> LGTM.


Thanks. I don't have the write access to the glibc repo. Could you
(someone else) apply the patch?

Yury


> On 02/18/2017 07:47 AM, Yury Norov wrote:

> > This patch adds tests for POSIX and Linux specific syscalls

> > that implemented with syscall templates machinery. The reason

> > of tests is to receive the expected error code and test if

> > it's handled properly by glibc.

> >

> > v1: https://sourceware.org/ml/libc-alpha/2017-02/msg00276.html

> > v2: added FSF copyright headers;

> >     support/test-driver.c is used instead of test-skeleton.c;

> >     fixed misspeling in description;

> >     'FAIL' prefix added in test wrapper macro fail paths.

> >

> > Yury Norov <ynorov@caviumnetworks.com>

> > Zack Weinberg <zackw@panix.com>

> >

> >         * posix/Makefile: Enable errno test for POSIX syscalls.

> >         * posix/test-errno.c: New file.

> >         * sysdeps/unix/sysv/linux/Makefile: Enable errno test for Linux syscalls.

> >         * sysdeps/unix/sysv/linux/test-errno.c: New file.

> >

> > ---

> >  posix/Makefile                       |   2 +-

> >  posix/test-errno.c                   | 153 +++++++++++++++++++++++++++++++++++

> >  sysdeps/unix/sysv/linux/Makefile     |   2 +-

> >  sysdeps/unix/sysv/linux/test-errno.c | 142 ++++++++++++++++++++++++++++++++

> >  4 files changed, 297 insertions(+), 2 deletions(-)

> >  create mode 100644 posix/test-errno.c

> >  create mode 100644 sysdeps/unix/sysv/linux/test-errno.c

> >

> > diff --git a/posix/Makefile b/posix/Makefile

> > index 8f23d64..ae17646 100644

> > --- a/posix/Makefile

> > +++ b/posix/Makefile

> > @@ -62,7 +62,7 @@ routines :=								      \

> >  	get_child_max sched_cpucount sched_cpualloc sched_cpufree

> >

> >  aux		:= init-posix environ

> > -tests		:= tstgetopt testfnm runtests runptests	     \

> > +tests		:= test-errno tstgetopt testfnm runtests runptests \

> >  		   tst-preadwrite tst-preadwrite64 test-vfork regexbug1 \

> >  		   tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \

> >  		   tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \

> > diff --git a/posix/test-errno.c b/posix/test-errno.c

> > new file mode 100644

> > index 0000000..d484d02

> > --- /dev/null

> > +++ b/posix/test-errno.c

> > @@ -0,0 +1,153 @@

> > +/* Test that failing system calls do set errno to the correct value.

> > +

> > +   Copyright (C) 2017 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 <errno.h>

> > +#include <limits.h>

> > +#include <grp.h>

> > +#include <fcntl.h>

> > +#include <stdio.h>

> > +#include <string.h>

> > +#include <sys/ioctl.h>

> > +#include <sys/socket.h>

> > +#include <sys/stat.h>

> > +#include <sys/time.h>

> > +#include <sys/resource.h>

> > +#include <sys/types.h>

> > +#include <sys/statfs.h>

> > +#include <sys/mman.h>

> > +#include <unistd.h>

> > +#include <netinet/in.h>

> > +

> > +/* This is not an exhaustive test: only system calls that can be

> > +   persuaded to fail with a consistent error code and no side effects

> > +   are included.  Usually these are failures due to invalid arguments,

> > +   with errno code EBADF or EINVAL.  The order of argument checks is

> > +   unspecified, so we must take care to provide arguments that only

> > +   allow _one_ failure mode.

> > +

> > +   Note that all system calls that can fail with EFAULT are permitted

> > +   to deliver a SIGSEGV signal instead, so we avoid supplying invalid

> > +   pointers in general, and we do not attempt to test system calls

> > +   that can only fail with EFAULT (e.g. gettimeofday, gethostname).

> > +

> > +   Also note that root-only system calls (e.g. acct, reboot) may, when

> > +   the test is run as an unprivileged user, fail due to insufficient

> > +   privileges before bothering to do argument checks, so those are not

> > +   tested either.

> > +

> > +   Also, system calls that take enum or a set of flags as argument is

> > +   not tested if POSIX doesn't specify exact binary values for all

> > +   flags, and so any value passed to flags may become valid.

> > +

> > +   Some tests assume "/bin/sh" names a file that exists and is not a

> > +   directory.  */

> > +

> > +#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\

> > +  (__extension__ ({						\

> > +    errno = 0xdead;						\

> > +    rtype ret = syscall (__VA_ARGS__);				\

> > +    int err = errno;						\

> > +    int fail;							\

> > +    if (ret == (rtype) -1 && err == experr)			\

> > +      fail = 0;							\

> > +    else							\

> > +      {								\

> > +        fail = 1;						\

> > +        if (ret != (rtype) -1)					\

> > +          printf ("FAIL: " #syscall ": didn't fail as expected"	\

> > +               " (return "prtype")\n", ret);			\

> > +        else if (err == 0xdead)					\

> > +          puts("FAIL: " #syscall ": didn't update errno\n");	\

> > +        else if (err != experr)					\

> > +          printf ("FAIL: " #syscall				\

> > +               ": errno is: %d (%s) expected: %d (%s)\n",	\

> > +               err, strerror (err), experr, strerror (experr));	\

> > +      }								\

> > +    fail;							\

> > +  }))

> > +

> > +#define test_wrp(experr, syscall, ...)				\

> > +  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)

> > +

> > +static int

> > +do_test (void)

> > +{

> > +  size_t pagesize = sysconf (_SC_PAGESIZE);

> > +  struct statfs sfs;

> > +  struct sockaddr sa;

> > +  socklen_t sl;

> > +  char buf[1];

> > +  struct iovec iov[1] = { { buf, 1 } };

> > +  struct sockaddr_in sin;

> > +  sin.sin_family = AF_INET;

> > +  sin.sin_port = htons (1026);

> > +  sin.sin_addr.s_addr = htonl (INADDR_LOOPBACK);

> > +  struct msghdr msg;

> > +  memset(&msg, 0, sizeof msg);

> > +  msg.msg_iov = iov;

> > +  msg.msg_iovlen = 1;

> > +

> > +  int fails = 0;

> > +  fails |= test_wrp (EBADF, accept, -1, &sa, &sl);

> > +  fails |= test_wrp (EINVAL, access, "/", -1);

> > +  fails |= test_wrp (EBADF, bind, -1, (struct sockaddr *)&sin, sizeof sin);

> > +  fails |= test_wrp (ENOTDIR, chdir, "/bin/sh");

> > +  fails |= test_wrp (EBADF, close, -1);

> > +  fails |= test_wrp (EBADF, connect, -1, (struct sockaddr *)&sin, sizeof sin);

> > +  fails |= test_wrp (EBADF, dup, -1);

> > +  fails |= test_wrp (EBADF, dup2, -1, -1);

> > +  fails |= test_wrp (EBADF, fchdir, -1);

> > +  fails |= test_wrp (EBADF, fchmod, -1, 0);

> > +  fails |= test_wrp (EBADF, fcntl, -1, 0);

> > +  fails |= test_wrp (EBADF, fstatfs, -1, &sfs);

> > +  fails |= test_wrp (EBADF, fsync, -1);

> > +  fails |= test_wrp (EBADF, ftruncate, -1, 0);

> > +  fails |= test_wrp (EINVAL, getgroups, -1, 0);

> > +  fails |= test_wrp (EBADF, getpeername, -1, &sa, &sl);

> > +  fails |= test_wrp (EBADF, getsockname, -1, &sa, &sl);

> > +  fails |= test_wrp (EBADF, getsockopt, -1, 0, 0, buf, &sl);

> > +  fails |= test_wrp (EBADF, ioctl, -1, TIOCNOTTY);

> > +  fails |= test_wrp (EBADF, listen, -1, 1);

> > +  fails |= test_wrp (EBADF, lseek, -1, 0, 0);

> > +  fails |= test_wrp (EINVAL, madvise, (void *) -1, -1, 0);

> > +  fails |= test_wrp_rv (void *, "%p", EBADF,

> > +                        mmap, 0, pagesize, PROT_READ, MAP_PRIVATE, -1, 0);

> > +  fails |= test_wrp (EINVAL, mprotect, (void *) -1, pagesize, -1);

> > +  fails |= test_wrp (EINVAL, msync, (void *) -1, pagesize, -1);

> > +  fails |= test_wrp (EINVAL, munmap, (void *) -1, 0);

> > +  fails |= test_wrp (EINVAL, open, "/bin/sh", -1, 0);

> > +  fails |= test_wrp (EBADF, read, -1, buf, 1);

> > +  fails |= test_wrp (EINVAL, readlink, "/", buf, -1);

> > +  fails |= test_wrp (EBADF, readv, -1, iov, 1);

> > +  fails |= test_wrp (EBADF, recv, -1, buf, 1, 0);

> > +  fails |= test_wrp (EBADF, recvfrom, -1, buf, 1, 0, &sa, &sl);

> > +  fails |= test_wrp (EBADF, recvmsg, -1, &msg, 0);

> > +  fails |= test_wrp (EINVAL, select, -1, 0, 0, 0, 0);

> > +  fails |= test_wrp (EBADF, send, -1, buf, 1, 0);

> > +  fails |= test_wrp (EBADF, sendmsg, -1, &msg, 0);

> > +  fails |= test_wrp (EBADF, sendto, -1, buf, 1, 0, &sa, sl);

> > +  fails |= test_wrp (EBADF, setsockopt, -1, 0, 0, buf, sl);

> > +  fails |= test_wrp (EBADF, shutdown, -1, SHUT_RD);

> > +  fails |= test_wrp (EBADF, write, -1, "Hello", sizeof ("Hello") );

> > +  fails |= test_wrp (EBADF, writev, -1, iov, 1 );

> > +

> > +  return fails;

> > +}

> > +

> > +#include "support/test-driver.c"

> > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile

> > index b3d6866..3c0c89f 100644

> > --- a/sysdeps/unix/sysv/linux/Makefile

> > +++ b/sysdeps/unix/sysv/linux/Makefile

> > @@ -43,7 +43,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \

> >  		  bits/mman-linux.h

> >

> >  tests += tst-clone tst-clone2 tst-fanotify tst-personality tst-quota \

> > -	 tst-sync_file_range

> > +	 tst-sync_file_range test-errno

> >

> >  # Generate the list of SYS_* macros for the system calls (__NR_* macros).

> >

> > diff --git a/sysdeps/unix/sysv/linux/test-errno.c b/sysdeps/unix/sysv/linux/test-errno.c

> > new file mode 100644

> > index 0000000..b768ff0

> > --- /dev/null

> > +++ b/sysdeps/unix/sysv/linux/test-errno.c

> > @@ -0,0 +1,142 @@

> > +/* Test that failing system calls do set errno to the correct value.

> > +

> > +   Copyright (C) 2017 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 <errno.h>

> > +#include <fcntl.h>

> > +#include <mqueue.h>

> > +#include <sched.h>

> > +#include <signal.h>

> > +#include <stdio.h>

> > +#include <string.h>

> > +#include <time.h>

> > +#include <unistd.h>

> > +#include <sys/epoll.h>

> > +#include <sys/eventfd.h>

> > +#include <sys/file.h>

> > +#include <sys/fsuid.h>

> > +#include <sys/inotify.h>

> > +#include <sys/mman.h>

> > +#include <sys/poll.h>

> > +#include <sys/quota.h>

> > +#include <sys/resource.h>

> > +#include <sys/select.h>

> > +#include <sys/sendfile.h>

> > +#include <sys/swap.h>

> > +#include <sys/time.h>

> > +#include <sys/types.h>

> > +#include <sys/wait.h>

> > +

> > +/* This is not an exhaustive test: only system calls that can be

> > +   persuaded to fail with a consistent error code and no side effects

> > +   are included.  Usually these are failures due to invalid arguments,

> > +   with errno code EBADF or EINVAL.  The order of argument checks is

> > +   unspecified, so we must take care to provide arguments that only

> > +   allow _one_ failure mode.

> > +

> > +   Note that all system calls that can fail with EFAULT are permitted

> > +   to deliver a SIGSEGV signal instead, so we avoid supplying invalid

> > +   pointers in general, and we do not attempt to test system calls

> > +   that can only fail with EFAULT (e.g. gettimeofday, gethostname).

> > +

> > +   Also note that root-only system calls (e.g. acct, reboot) may, when

> > +   the test is run as an unprivileged user, fail due to insufficient

> > +   privileges before bothering to do argument checks, so those are not

> > +   tested either.

> > +

> > +   Also, system calls that take enum or a set of flags as argument is

> > +   not tested if POSIX doesn't specify exact binary values for all

> > +   flags, and so any value passed to flags may become valid.

> > +

> > +   Some tests assume "/bin/sh" names a file that exists and is not a

> > +   directory.  */

> > +

> > +#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\

> > +  (__extension__ ({						\

> > +    errno = 0xdead;						\

> > +    rtype ret = syscall (__VA_ARGS__);				\

> > +    int err = errno;						\

> > +    int fail;							\

> > +    if (ret == (rtype) -1 && err == experr)			\

> > +      fail = 0;							\

> > +    else							\

> > +      {								\

> > +        fail = 1;						\

> > +        if (ret != (rtype) -1)					\

> > +          printf ("FAIL: " #syscall ": didn't fail as expected"	\

> > +               " (return "prtype")\n", ret);			\

> > +        else if (err == 0xdead)					\

> > +          puts("FAIL: " #syscall ": didn't update errno\n");	\

> > +        else if (err != experr)					\

> > +          printf ("FAIL: " #syscall				\

> > +               ": errno is: %d (%s) expected: %d (%s)\n",	\

> > +               err, strerror (err), experr, strerror (experr));	\

> > +      }								\

> > +    fail;							\

> > +  }))

> > +

> > +#define test_wrp(experr, syscall, ...)				\

> > +  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)

> > +

> > +static int

> > +do_test (void)

> > +{

> > +  fd_set rs, ws, es;

> > +  int status;

> > +  off_t off;

> > +  stack_t ss;

> > +  struct dqblk dqblk;

> > +  struct epoll_event epoll_event;

> > +  struct pollfd pollfd;

> > +  struct sched_param sch_param;

> > +  struct timespec ts;

> > +  struct timeval tv;

> > +  unsigned char vec[16];

> > +  ss.ss_flags = ~SS_DISABLE;

> > +  ts.tv_sec = -1;

> > +

> > +  int fails = 0;

> > +  fails |= test_wrp (EINVAL, epoll_create, -1);

> > +  fails |= test_wrp (EINVAL, epoll_create1, EPOLL_CLOEXEC + 1);

> > +  fails |= test_wrp (EBADF, epoll_ctl, -1, EPOLL_CTL_ADD, 0, &epoll_event);

> > +  fails |= test_wrp (EBADF, epoll_wait, -1, &epoll_event, 1, 1);

> > +  fails |= test_wrp (EBADF, fdatasync, -1);

> > +  fails |= test_wrp (EBADF, flock, -1, LOCK_SH);

> > +  fails |= test_wrp (ESRCH, getpgid, -1);

> > +  fails |= test_wrp (EINVAL, inotify_add_watch, -1, "/", 0);

> > +  fails |= test_wrp (EINVAL, mincore, (void *) -1, 0, vec);

> > +  fails |= test_wrp (EINVAL, mlock, (void *) -1, 1); // different errors

> > +  fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);

> > +  fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);

> > +  fails |= test_wrp (ENODEV, quotactl, Q_GETINFO, NULL, -1, (caddr_t) &dqblk);

> > +  fails |= test_wrp (EINVAL, sched_getparam, -1, &sch_param);

> > +  fails |= test_wrp (EINVAL, sched_getscheduler, -1);

> > +  fails |= test_wrp (EINVAL, sched_get_priority_max, -1);

> > +  fails |= test_wrp (EINVAL, sched_get_priority_min, -1);

> > +  fails |= test_wrp (EINVAL, sched_rr_get_interval, -1, &ts);

> > +  fails |= test_wrp (EINVAL, sched_setparam, -1, &sch_param);

> > +  fails |= test_wrp (EINVAL, sched_setscheduler, -1, 0, &sch_param);

> > +  fails |= test_wrp (EINVAL, select, -1, &rs, &ws, &es, &tv);

> > +  fails |= test_wrp (EBADF, sendfile, -1, -1, &off, 0);

> > +  fails |= test_wrp (EINVAL, sigaltstack, &ss, NULL);

> > +  fails |= test_wrp (ECHILD, wait4, -1, &status, 0, NULL);

> > +

> > +  return fails;

> > +}

> > +

> > +#include "support/test-driver.c"
Zack Weinberg March 8, 2017, 4:20 p.m. | #4
On 03/06/2017 09:42 PM, Yury Norov wrote:
> On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta wrote:

>> LGTM.

> Thanks. I don't have the write access to the glibc repo. Could you

> (someone else) apply the patch?


I have committed the patch.

zw
Stefan Liebler March 9, 2017, 3 p.m. | #5
On 03/08/2017 05:20 PM, Zack Weinberg wrote:
> On 03/06/2017 09:42 PM, Yury Norov wrote:

>> On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta wrote:

>>> LGTM.

>> Thanks. I don't have the write access to the glibc repo. Could you

>> (someone else) apply the patch?

>

> I have committed the patch.

>

> zw

>

>

Hi,

on s390 (31bit), I get the following fails:
FAIL: misc/test-errno:
FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid 
argument)

FAIL: posix/test-errno:
FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid 
argument)

Is it intended, that the same test is run twice?
Both are compiled with sysdeps/unix/sysv/linux/test-errno.c.
Or should there two different tests, one compiled
with posix/test-errno.c
and the other with sysdeps/unix/sysv/linux/test-errno.c?

Why is the test-errno added to tests in sysdeps/unix/sysv/linux/Makefile 
with:
ifeq ($(subdir),misc)
tests += test-errno
endif



Regarding mlock-syscall:
If the compat mlock syscall is used, it returns 12 (ENOMEM).
This is also observable if you compile and run the testcase with -m32 on 
a x86_64 system.


I've compiled and run posix/test-errno.c on my s390x system and
get the following error:
FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file 
descriptor)

sl=0xfdfa9170 before setsockopt syscall.
The test succeeds if I sl is initialized to zero.

Bye
Stefan
Florian Weimer March 9, 2017, 4:18 p.m. | #6
On 03/09/2017 04:00 PM, Stefan Liebler wrote:
> On 03/08/2017 05:20 PM, Zack Weinberg wrote:

>> On 03/06/2017 09:42 PM, Yury Norov wrote:

>>> On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta

>>> wrote:

>>>> LGTM.

>>> Thanks. I don't have the write access to the glibc repo. Could you

>>> (someone else) apply the patch?

>>

>> I have committed the patch.

>>

>> zw

>>

>>

> Hi,

>

> on s390 (31bit), I get the following fails:

> FAIL: misc/test-errno:

> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid

> argument)

>

> FAIL: posix/test-errno:

> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid

> argument)

>

> Is it intended, that the same test is run twice?


Good catch.  I think the Linux test should be renamed, so that it does 
not override the generic test.  We want to run both on Linux.

> Why is the test-errno added to tests in sysdeps/unix/sysv/linux/Makefile

> with:

> ifeq ($(subdir),misc)

> tests += test-errno

> endif


After renaming the source file, something like that will be necessary to 
run the test.

> Regarding mlock-syscall:

> If the compat mlock syscall is used, it returns 12 (ENOMEM).

> This is also observable if you compile and run the testcase with -m32 on

> a x86_64 system.


I'd argue that this is a kernel bug.  EINVAL is required here because 
the address computation overflows (in 32-bit mode).  I don't see a good 
way to force a different error code, without risking thrashing the 
system.  Maybe we could map a page, unmap it, and pass that to mlock? 
That should result in ENOMEM reliably, I think.

> I've compiled and run posix/test-errno.c on my s390x system and

> get the following error:

> FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file

> descriptor)

> sl=0xfdfa9170 before setsockopt syscall.

> The test succeeds if I sl is initialized to zero.


Right, the initializer is missing.  Assuming that none of the get* calls 
which are called with &sl modify the variable (which is a reasonable 
assumption because they are expected to fail), adding the initializer 
should be the right fix.

Thanks,
Florian
Adhemerval Zanella March 9, 2017, 4:30 p.m. | #7
On 09/03/2017 16:00, Stefan Liebler wrote:
> On 03/08/2017 05:20 PM, Zack Weinberg wrote:

>> On 03/06/2017 09:42 PM, Yury Norov wrote:

>>> On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta wrote:

>>>> LGTM.

>>> Thanks. I don't have the write access to the glibc repo. Could you

>>> (someone else) apply the patch?

>>

>> I have committed the patch.

>>

>> zw

>>

>>

> Hi,

> 

> on s390 (31bit), I get the following fails:

> FAIL: misc/test-errno:

> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid argument)

> 

> FAIL: posix/test-errno:

> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid argument)

> 

> Is it intended, that the same test is run twice?


It is since although they have the same name they are different test
in fact: posix/test-errno.c only uses POSIX defined interfaces while
sysdeps/unix/sysv/linux/test-errno.c check for Linux specific ones.

> Both are compiled with sysdeps/unix/sysv/linux/test-errno.c.


But this indeed the issue and it need to be fixed. I am not sure if
glibc Makefile system can handle test with same name in multiple
paths (and I personally not compiling to actually debug if it is
the case), so I would recommend to just rename Linux specific one
to test-errno-linux.c.

> Or should there two different tests, one compiled

> with posix/test-errno.c

> and the other with sysdeps/unix/sysv/linux/test-errno.c?

> 

> Why is the test-errno added to tests in sysdeps/unix/sysv/linux/Makefile with:

> ifeq ($(subdir),misc)

> tests += test-errno

> endif


Afaik it is basically to put the resulting objects/binaries on misc folder
(where usually Linux-only tests are placed).

> 

> 

> 

> Regarding mlock-syscall:

> If the compat mlock syscall is used, it returns 12 (ENOMEM).

> This is also observable if you compile and run the testcase with -m32 on a x86_64 system.

> 

> 

> I've compiled and run posix/test-errno.c on my s390x system and

> get the following error:

> FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file descriptor)

> 

> sl=0xfdfa9170 before setsockopt syscall.

> The test succeeds if I sl is initialized to zero.

> 


POSIX [1] states that mlock should may fail with EINVAL only if
the addr argument is not a multiple of {PAGESIZE}.  Linux does
not return this issue, since it aligns the resulting address
to pagesize:

* mm/mlock.c:

666 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
667 {
[...]
676 
677         len = PAGE_ALIGN(len + (offset_in_page(start)));
678         start &= PAGE_MASK;

EINVAL is only returned on 'apply_vma_lock_flags':

578 static int apply_vma_lock_flags(unsigned long start, size_t len,
579                                 vm_flags_t flags)
580 {
[...]
587         end = start + len;
588         if (end < start)
589                 return -EINVAL;

But if you runs 32 binaries on 64 bits kernel overflow won't happen.
It is documented in man pages, so I think from kernel side it should
be consistent for 32 bit binaries on 64 bit kernel.

For glibc side, I think we should do something like:


[1] http://pubs.opengroup.org/onlinepubs/9699919799/

> Bye

> Stefan

>diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile

index 6b7aa3f..1872cdb 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/mman-linux.h
 
 tests += tst-clone tst-clone2 tst-fanotify tst-personality tst-quota \
-	 tst-sync_file_range test-errno
+	 tst-sync_file_range test-errno-linux
 
 # Generate the list of SYS_* macros for the system calls (__NR_* macros).
 
diff --git a/sysdeps/unix/sysv/linux/test-errno-linux.c b/sysdeps/unix/sysv/linux/test-errno-linux.c
index ab3735f..c3facd5 100644
--- a/sysdeps/unix/sysv/linux/test-errno-linux.c
+++ b/sysdeps/unix/sysv/linux/test-errno-linux.c
@@ -1,4 +1,5 @@
 /* Test that failing system calls do set errno to the correct value.
+   Linux sycalls version.
 
    Copyright (C) 2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
@@ -90,9 +91,37 @@
     fail;							\
   }))
 
+#define test_wrp_rv2(rtype, prtype, experr1, experr2, syscall, ...) 	\
+  (__extension__ ({							\
+    errno = 0xdead;							\
+    rtype ret = syscall (__VA_ARGS__);					\
+    int err = errno;							\
+    int fail;								\
+    if (ret == (rtype) -1 && (err == experr1 || err == experr2))	\
+      fail = 0;								\
+    else								\
+      {									\
+        fail = 1;							\
+        if (ret != (rtype) -1)						\
+          printf ("FAIL: " #syscall ": didn't fail as expected"		\
+               " (return "prtype")\n", ret);				\
+        else if (err == 0xdead)						\
+          puts("FAIL: " #syscall ": didn't update errno\n");		\
+        else if (err != experr1 && err != experr2)			\
+          printf ("FAIL: " #syscall					\
+               ": errno is: %d (%s) expected: %d (%s) or %d (%s)\n",	\
+               err, strerror (err), experr1, strerror (experr1),	\
+               experr2, strerror (experr2));				\
+      }									\
+    fail;								\
+  }))
+
 #define test_wrp(experr, syscall, ...)				\
   test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)
 
+#define test_wrp2(experr1, experr2, syscall, ...)		\
+  test_wrp_rv2(int, "%d", experr1, experr2, syscall, __VA_ARGS__)
+
 static int
 do_test (void)
 {
@@ -120,7 +149,12 @@ do_test (void)
   fails |= test_wrp (ESRCH, getpgid, -1);
   fails |= test_wrp (EINVAL, inotify_add_watch, -1, "/", 0);
   fails |= test_wrp (EINVAL, mincore, (void *) -1, 0, vec);
-  fails |= test_wrp (EINVAL, mlock, (void *) -1, 1); // different errors
+  /* mlock fails if the result of the addition addr+len was less than addr
+     which indicates final address overflow), however on 32 bits binaries
+     running on 64 bits kernel, internal syscall address check won't result
+     in an invalid address and thus syscalls fails later in vma
+     allocation.  */
+  fails |= test_wrp2 (EINVAL, ENOMEM, mlock, (void *) -1, 1);
   fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
   fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
   fails |= test_wrp (ENODEV, quotactl, Q_GETINFO, NULL, -1, (caddr_t) &dqblk);

Yury Norov March 9, 2017, 10:29 p.m. | #8
On Thu, Mar 09, 2017 at 05:30:03PM +0100, Adhemerval Zanella wrote:
> > Regarding mlock-syscall:

> > If the compat mlock syscall is used, it returns 12 (ENOMEM).

> > This is also observable if you compile and run the testcase with -m32 on a x86_64 system.

> > 

> > 

> > I've compiled and run posix/test-errno.c on my s390x system and

> > get the following error:

> > FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file descriptor)

> > 

> > sl=0xfdfa9170 before setsockopt syscall.

> > The test succeeds if I sl is initialized to zero.

> > 

> 

> POSIX [1] states that mlock should may fail with EINVAL only if

> the addr argument is not a multiple of {PAGESIZE}.  Linux does

> not return this issue, since it aligns the resulting address

> to pagesize:

> 

> * mm/mlock.c:

> 

> 666 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)

> 667 {

> [...]

> 676 

> 677         len = PAGE_ALIGN(len + (offset_in_page(start)));

> 678         start &= PAGE_MASK;

> 

> EINVAL is only returned on 'apply_vma_lock_flags':

> 

> 578 static int apply_vma_lock_flags(unsigned long start, size_t len,

> 579                                 vm_flags_t flags)

> 580 {

> [...]

> 587         end = start + len;

> 588         if (end < start)

> 589                 return -EINVAL;

> 

> But if you runs 32 binaries on 64 bits kernel overflow won't happen.

> It is documented in man pages, so I think from kernel side it should

> be consistent for 32 bit binaries on 64 bit kernel.


I also observed this fail in aarch64/ilp32, but I decided that this
is the ilp32 problem while it's compat one, as you discovered here.

The test is not intended to check each and every syscall, as it is
mentioned in comment. For example, all syscalls that may fail with
EFAULT are not tested because they may both return the EFAULT or
actually terminate the application with segfault. This is the
similar case for me. I think that it's better to keep the test simple,
and just drop the mlock test, and not introduce another macro.

Though, the patch that handles the issue is already there, and I don't
object if you submit it.

Yury
Adhemerval Zanella March 13, 2017, 12:54 p.m. | #9
On 09/03/2017 19:29, Yury Norov wrote:
> On Thu, Mar 09, 2017 at 05:30:03PM +0100, Adhemerval Zanella wrote:

>>> Regarding mlock-syscall:

>>> If the compat mlock syscall is used, it returns 12 (ENOMEM).

>>> This is also observable if you compile and run the testcase with -m32 on a x86_64 system.

>>>

>>>

>>> I've compiled and run posix/test-errno.c on my s390x system and

>>> get the following error:

>>> FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file descriptor)

>>>

>>> sl=0xfdfa9170 before setsockopt syscall.

>>> The test succeeds if I sl is initialized to zero.

>>>

>>

>> POSIX [1] states that mlock should may fail with EINVAL only if

>> the addr argument is not a multiple of {PAGESIZE}.  Linux does

>> not return this issue, since it aligns the resulting address

>> to pagesize:

>>

>> * mm/mlock.c:

>>

>> 666 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)

>> 667 {

>> [...]

>> 676 

>> 677         len = PAGE_ALIGN(len + (offset_in_page(start)));

>> 678         start &= PAGE_MASK;

>>

>> EINVAL is only returned on 'apply_vma_lock_flags':

>>

>> 578 static int apply_vma_lock_flags(unsigned long start, size_t len,

>> 579                                 vm_flags_t flags)

>> 580 {

>> [...]

>> 587         end = start + len;

>> 588         if (end < start)

>> 589                 return -EINVAL;

>>

>> But if you runs 32 binaries on 64 bits kernel overflow won't happen.

>> It is documented in man pages, so I think from kernel side it should

>> be consistent for 32 bit binaries on 64 bit kernel.

> 

> I also observed this fail in aarch64/ilp32, but I decided that this

> is the ilp32 problem while it's compat one, as you discovered here.

> 

> The test is not intended to check each and every syscall, as it is

> mentioned in comment. For example, all syscalls that may fail with

> EFAULT are not tested because they may both return the EFAULT or

> actually terminate the application with segfault. This is the

> similar case for me. I think that it's better to keep the test simple,

> and just drop the mlock test, and not introduce another macro.

> 

> Though, the patch that handles the issue is already there, and I don't

> object if you submit it.


I do not have a strong opinion here, but since adjusting to check for
both results is pretty much straightforward I think it is worth to just
add it and comment why it requires to check for both results.  I will
send a patch upstream shortly.

Patch hide | download patch | download mbox

diff --git a/posix/Makefile b/posix/Makefile
index 8f23d64..ae17646 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -62,7 +62,7 @@  routines :=								      \
 	get_child_max sched_cpucount sched_cpualloc sched_cpufree
 
 aux		:= init-posix environ
-tests		:= tstgetopt testfnm runtests runptests	     \
+tests		:= test-errno tstgetopt testfnm runtests runptests \
 		   tst-preadwrite tst-preadwrite64 test-vfork regexbug1 \
 		   tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \
 		   tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \
diff --git a/posix/test-errno.c b/posix/test-errno.c
new file mode 100644
index 0000000..d484d02
--- /dev/null
+++ b/posix/test-errno.c
@@ -0,0 +1,153 @@ 
+/* Test that failing system calls do set errno to the correct value.
+
+   Copyright (C) 2017 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 <errno.h>
+#include <limits.h>
+#include <grp.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <sys/types.h>
+#include <sys/statfs.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <netinet/in.h>
+
+/* This is not an exhaustive test: only system calls that can be
+   persuaded to fail with a consistent error code and no side effects
+   are included.  Usually these are failures due to invalid arguments,
+   with errno code EBADF or EINVAL.  The order of argument checks is
+   unspecified, so we must take care to provide arguments that only
+   allow _one_ failure mode.
+
+   Note that all system calls that can fail with EFAULT are permitted
+   to deliver a SIGSEGV signal instead, so we avoid supplying invalid
+   pointers in general, and we do not attempt to test system calls
+   that can only fail with EFAULT (e.g. gettimeofday, gethostname).
+
+   Also note that root-only system calls (e.g. acct, reboot) may, when
+   the test is run as an unprivileged user, fail due to insufficient
+   privileges before bothering to do argument checks, so those are not
+   tested either.
+
+   Also, system calls that take enum or a set of flags as argument is
+   not tested if POSIX doesn't specify exact binary values for all
+   flags, and so any value passed to flags may become valid.
+
+   Some tests assume "/bin/sh" names a file that exists and is not a
+   directory.  */
+
+#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\
+  (__extension__ ({						\
+    errno = 0xdead;						\
+    rtype ret = syscall (__VA_ARGS__);				\
+    int err = errno;						\
+    int fail;							\
+    if (ret == (rtype) -1 && err == experr)			\
+      fail = 0;							\
+    else							\
+      {								\
+        fail = 1;						\
+        if (ret != (rtype) -1)					\
+          printf ("FAIL: " #syscall ": didn't fail as expected"	\
+               " (return "prtype")\n", ret);			\
+        else if (err == 0xdead)					\
+          puts("FAIL: " #syscall ": didn't update errno\n");	\
+        else if (err != experr)					\
+          printf ("FAIL: " #syscall				\
+               ": errno is: %d (%s) expected: %d (%s)\n",	\
+               err, strerror (err), experr, strerror (experr));	\
+      }								\
+    fail;							\
+  }))
+
+#define test_wrp(experr, syscall, ...)				\
+  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)
+
+static int
+do_test (void)
+{
+  size_t pagesize = sysconf (_SC_PAGESIZE);
+  struct statfs sfs;
+  struct sockaddr sa;
+  socklen_t sl;
+  char buf[1];
+  struct iovec iov[1] = { { buf, 1 } };
+  struct sockaddr_in sin;
+  sin.sin_family = AF_INET;
+  sin.sin_port = htons (1026);
+  sin.sin_addr.s_addr = htonl (INADDR_LOOPBACK);
+  struct msghdr msg;
+  memset(&msg, 0, sizeof msg);
+  msg.msg_iov = iov;
+  msg.msg_iovlen = 1;
+
+  int fails = 0;
+  fails |= test_wrp (EBADF, accept, -1, &sa, &sl);
+  fails |= test_wrp (EINVAL, access, "/", -1);
+  fails |= test_wrp (EBADF, bind, -1, (struct sockaddr *)&sin, sizeof sin);
+  fails |= test_wrp (ENOTDIR, chdir, "/bin/sh");
+  fails |= test_wrp (EBADF, close, -1);
+  fails |= test_wrp (EBADF, connect, -1, (struct sockaddr *)&sin, sizeof sin);
+  fails |= test_wrp (EBADF, dup, -1);
+  fails |= test_wrp (EBADF, dup2, -1, -1);
+  fails |= test_wrp (EBADF, fchdir, -1);
+  fails |= test_wrp (EBADF, fchmod, -1, 0);
+  fails |= test_wrp (EBADF, fcntl, -1, 0);
+  fails |= test_wrp (EBADF, fstatfs, -1, &sfs);
+  fails |= test_wrp (EBADF, fsync, -1);
+  fails |= test_wrp (EBADF, ftruncate, -1, 0);
+  fails |= test_wrp (EINVAL, getgroups, -1, 0);
+  fails |= test_wrp (EBADF, getpeername, -1, &sa, &sl);
+  fails |= test_wrp (EBADF, getsockname, -1, &sa, &sl);
+  fails |= test_wrp (EBADF, getsockopt, -1, 0, 0, buf, &sl);
+  fails |= test_wrp (EBADF, ioctl, -1, TIOCNOTTY);
+  fails |= test_wrp (EBADF, listen, -1, 1);
+  fails |= test_wrp (EBADF, lseek, -1, 0, 0);
+  fails |= test_wrp (EINVAL, madvise, (void *) -1, -1, 0);
+  fails |= test_wrp_rv (void *, "%p", EBADF,
+                        mmap, 0, pagesize, PROT_READ, MAP_PRIVATE, -1, 0);
+  fails |= test_wrp (EINVAL, mprotect, (void *) -1, pagesize, -1);
+  fails |= test_wrp (EINVAL, msync, (void *) -1, pagesize, -1);
+  fails |= test_wrp (EINVAL, munmap, (void *) -1, 0);
+  fails |= test_wrp (EINVAL, open, "/bin/sh", -1, 0);
+  fails |= test_wrp (EBADF, read, -1, buf, 1);
+  fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
+  fails |= test_wrp (EBADF, readv, -1, iov, 1);
+  fails |= test_wrp (EBADF, recv, -1, buf, 1, 0);
+  fails |= test_wrp (EBADF, recvfrom, -1, buf, 1, 0, &sa, &sl);
+  fails |= test_wrp (EBADF, recvmsg, -1, &msg, 0);
+  fails |= test_wrp (EINVAL, select, -1, 0, 0, 0, 0);
+  fails |= test_wrp (EBADF, send, -1, buf, 1, 0);
+  fails |= test_wrp (EBADF, sendmsg, -1, &msg, 0);
+  fails |= test_wrp (EBADF, sendto, -1, buf, 1, 0, &sa, sl);
+  fails |= test_wrp (EBADF, setsockopt, -1, 0, 0, buf, sl);
+  fails |= test_wrp (EBADF, shutdown, -1, SHUT_RD);
+  fails |= test_wrp (EBADF, write, -1, "Hello", sizeof ("Hello") );
+  fails |= test_wrp (EBADF, writev, -1, iov, 1 );
+
+  return fails;
+}
+
+#include "support/test-driver.c"
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index b3d6866..3c0c89f 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,7 @@  sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/mman-linux.h
 
 tests += tst-clone tst-clone2 tst-fanotify tst-personality tst-quota \
-	 tst-sync_file_range
+	 tst-sync_file_range test-errno
 
 # Generate the list of SYS_* macros for the system calls (__NR_* macros).
 
diff --git a/sysdeps/unix/sysv/linux/test-errno.c b/sysdeps/unix/sysv/linux/test-errno.c
new file mode 100644
index 0000000..b768ff0
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/test-errno.c
@@ -0,0 +1,142 @@ 
+/* Test that failing system calls do set errno to the correct value.
+
+   Copyright (C) 2017 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 <errno.h>
+#include <fcntl.h>
+#include <mqueue.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/epoll.h>
+#include <sys/eventfd.h>
+#include <sys/file.h>
+#include <sys/fsuid.h>
+#include <sys/inotify.h>
+#include <sys/mman.h>
+#include <sys/poll.h>
+#include <sys/quota.h>
+#include <sys/resource.h>
+#include <sys/select.h>
+#include <sys/sendfile.h>
+#include <sys/swap.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+/* This is not an exhaustive test: only system calls that can be
+   persuaded to fail with a consistent error code and no side effects
+   are included.  Usually these are failures due to invalid arguments,
+   with errno code EBADF or EINVAL.  The order of argument checks is
+   unspecified, so we must take care to provide arguments that only
+   allow _one_ failure mode.
+
+   Note that all system calls that can fail with EFAULT are permitted
+   to deliver a SIGSEGV signal instead, so we avoid supplying invalid
+   pointers in general, and we do not attempt to test system calls
+   that can only fail with EFAULT (e.g. gettimeofday, gethostname).
+
+   Also note that root-only system calls (e.g. acct, reboot) may, when
+   the test is run as an unprivileged user, fail due to insufficient
+   privileges before bothering to do argument checks, so those are not
+   tested either.
+
+   Also, system calls that take enum or a set of flags as argument is
+   not tested if POSIX doesn't specify exact binary values for all
+   flags, and so any value passed to flags may become valid.
+
+   Some tests assume "/bin/sh" names a file that exists and is not a
+   directory.  */
+
+#define test_wrp_rv(rtype, prtype, experr, syscall, ...)	\
+  (__extension__ ({						\
+    errno = 0xdead;						\
+    rtype ret = syscall (__VA_ARGS__);				\
+    int err = errno;						\
+    int fail;							\
+    if (ret == (rtype) -1 && err == experr)			\
+      fail = 0;							\
+    else							\
+      {								\
+        fail = 1;						\
+        if (ret != (rtype) -1)					\
+          printf ("FAIL: " #syscall ": didn't fail as expected"	\
+               " (return "prtype")\n", ret);			\
+        else if (err == 0xdead)					\
+          puts("FAIL: " #syscall ": didn't update errno\n");	\
+        else if (err != experr)					\
+          printf ("FAIL: " #syscall				\
+               ": errno is: %d (%s) expected: %d (%s)\n",	\
+               err, strerror (err), experr, strerror (experr));	\
+      }								\
+    fail;							\
+  }))
+
+#define test_wrp(experr, syscall, ...)				\
+  test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)
+
+static int
+do_test (void)
+{
+  fd_set rs, ws, es;
+  int status;
+  off_t off;
+  stack_t ss;
+  struct dqblk dqblk;
+  struct epoll_event epoll_event;
+  struct pollfd pollfd;
+  struct sched_param sch_param;
+  struct timespec ts;
+  struct timeval tv;
+  unsigned char vec[16];
+  ss.ss_flags = ~SS_DISABLE;
+  ts.tv_sec = -1;
+
+  int fails = 0;
+  fails |= test_wrp (EINVAL, epoll_create, -1);
+  fails |= test_wrp (EINVAL, epoll_create1, EPOLL_CLOEXEC + 1);
+  fails |= test_wrp (EBADF, epoll_ctl, -1, EPOLL_CTL_ADD, 0, &epoll_event);
+  fails |= test_wrp (EBADF, epoll_wait, -1, &epoll_event, 1, 1);
+  fails |= test_wrp (EBADF, fdatasync, -1);
+  fails |= test_wrp (EBADF, flock, -1, LOCK_SH);
+  fails |= test_wrp (ESRCH, getpgid, -1);
+  fails |= test_wrp (EINVAL, inotify_add_watch, -1, "/", 0);
+  fails |= test_wrp (EINVAL, mincore, (void *) -1, 0, vec);
+  fails |= test_wrp (EINVAL, mlock, (void *) -1, 1); // different errors
+  fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
+  fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
+  fails |= test_wrp (ENODEV, quotactl, Q_GETINFO, NULL, -1, (caddr_t) &dqblk);
+  fails |= test_wrp (EINVAL, sched_getparam, -1, &sch_param);
+  fails |= test_wrp (EINVAL, sched_getscheduler, -1);
+  fails |= test_wrp (EINVAL, sched_get_priority_max, -1);
+  fails |= test_wrp (EINVAL, sched_get_priority_min, -1);
+  fails |= test_wrp (EINVAL, sched_rr_get_interval, -1, &ts);
+  fails |= test_wrp (EINVAL, sched_setparam, -1, &sch_param);
+  fails |= test_wrp (EINVAL, sched_setscheduler, -1, 0, &sch_param);
+  fails |= test_wrp (EINVAL, select, -1, &rs, &ws, &es, &tv);
+  fails |= test_wrp (EBADF, sendfile, -1, -1, &off, 0);
+  fails |= test_wrp (EINVAL, sigaltstack, &ss, NULL);
+  fails |= test_wrp (ECHILD, wait4, -1, &status, 0, NULL);
+
+  return fails;
+}
+
+#include "support/test-driver.c"