This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix misreported errno on preadv2/pwritev2 (BZ#23579)
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Thu, 20 Sep 2018 11:32:50 -0700
- Subject: Re: [PATCH] Fix misreported errno on preadv2/pwritev2 (BZ#23579)
- References: <20180829212032.32507-1-adhemerval.zanella@linaro.org>
Ping.
On 29/08/2018 14:20, Adhemerval Zanella wrote:
> The fallback code of Linux wrapper for preadv2/pwritev2 executes
> regardless of the errno code for preadv2, instead of the case where
> the syscall is not supported.
>
> This fixes it by calling the fallback code iff errno is ENOSYS. The
> patch also adds tests for both invalid file descriptor and invalid
> iov_len and vector count.
>
> The only discrepancy between preadv2 and fallback code regarding
> error reporting is when an invalid flags are used. The fallback code
> bails out earlier with ENOTSUP instead of EINVAL/EBADF when the syscall
> is used.
>
> Checked on x86_64-linux-gnu on a 4.4.0 and 4.15.0 kernel.
>
> [BZ #23579]
> * misc/tst-preadvwritev2-common.c (do_test_with_invalid_fd): New
> test.
> * misc/tst-preadvwritev2.c, misc/tst-preadvwritev64v2.c (do_test):
> Call do_test_with_invalid_fd.
> * sysdeps/unix/sysv/linux/preadv2.c (preadv2): Use fallback code iff
> errno is ENOSYS.
> * sysdeps/unix/sysv/linux/preadv64v2.c (preadv64v2): Likewise.
> * sysdeps/unix/sysv/linux/pwritev2.c (pwritev2): Likewise.
> * sysdeps/unix/sysv/linux/pwritev64v2.c (pwritev64v2): Likewise.
> ---
> ChangeLog | 13 ++++++
> misc/tst-preadvwritev2-common.c | 65 +++++++++++++++++++++++++--
> misc/tst-preadvwritev2.c | 2 +
> misc/tst-preadvwritev64v2.c | 2 +
> sysdeps/unix/sysv/linux/preadv2.c | 2 +-
> sysdeps/unix/sysv/linux/preadv64v2.c | 2 +-
> sysdeps/unix/sysv/linux/pwritev2.c | 2 +-
> sysdeps/unix/sysv/linux/pwritev64v2.c | 2 +-
> 8 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/misc/tst-preadvwritev2-common.c b/misc/tst-preadvwritev2-common.c
> index f889a21544..50b9da3fea 100644
> --- a/misc/tst-preadvwritev2-common.c
> +++ b/misc/tst-preadvwritev2-common.c
> @@ -19,9 +19,6 @@
> #include <limits.h>
> #include <support/check.h>
>
> -static void
> -do_test_with_invalid_flags (void)
> -{
> #ifndef RWF_HIPRI
> # define RWF_HIPRI 0
> #endif
> @@ -39,6 +36,68 @@ do_test_with_invalid_flags (void)
> #endif
> #define RWF_SUPPORTED (RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT \
> | RWF_APPEND)
> +
> +static void
> +do_test_with_invalid_fd (void)
> +{
> + char buf[256];
> + struct iovec iov = { buf, sizeof buf };
> +
> + /* Check with flag being 0 to use the fallback code which calls pwritev
> + or writev. */
> + TEST_VERIFY (preadv2 (-1, &iov, 1, -1, 0) == -1);
> + TEST_COMPARE (errno, EBADF);
> + TEST_VERIFY (pwritev2 (-1, &iov, 1, -1, 0) == -1);
> + TEST_COMPARE (errno, EBADF);
> +
> + /* Same tests as before but with flags being different than 0. Since
> + there is no emulation for any flag value, fallback code returns
> + ENOTSUP. This is different running on a kernel with preadv2/pwritev2
> + support, where EBADF is returned). */
> + TEST_VERIFY (preadv2 (-1, &iov, 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EBADF || errno == ENOTSUP);
> + TEST_VERIFY (pwritev2 (-1, &iov, 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EBADF || errno == ENOTSUP);
> +}
> +
> +static void
> +do_test_with_invalid_iov (void)
> +{
> + {
> + char buf[256];
> + struct iovec iov;
> +
> + iov.iov_base = buf;
> + iov.iov_len = (size_t)SSIZE_MAX + 1;
> +
> + TEST_VERIFY (preadv2 (temp_fd, &iov, 1, 0, 0) == -1);
> + TEST_COMPARE (errno, EINVAL);
> + TEST_VERIFY (pwritev2 (temp_fd, &iov, 1, 0, 0) == -1);
> + TEST_COMPARE (errno, EINVAL);
> +
> + /* Same as for invalid file descriptor tests, emulation fallback
> + first checks for flag value and return ENOTSUP. */
> + TEST_VERIFY (preadv2 (temp_fd, &iov, 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EINVAL || errno == ENOTSUP);
> + TEST_VERIFY (pwritev2 (temp_fd, &iov, 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EINVAL || errno == ENOTSUP);
> + }
> +
> + {
> + /* An invalid iovec buffer should trigger an invalid memory access
> + or an error (Linux for instance returns EFAULT). */
> + struct iovec iov[IOV_MAX+1] = { 0 };
> +
> + TEST_VERIFY (preadv2 (temp_fd, iov, IOV_MAX + 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EINVAL || errno == ENOTSUP);
> + TEST_VERIFY (pwritev2 (temp_fd, iov, IOV_MAX + 1, 0, RWF_HIPRI) == -1);
> + TEST_VERIFY (errno == EINVAL || errno == ENOTSUP);
> + }
> +}
> +
> +static void
> +do_test_with_invalid_flags (void)
> +{
> /* Set the next bit from the mask of all supported flags. */
> int invalid_flag = RWF_SUPPORTED != 0 ? __builtin_clz (RWF_SUPPORTED) : 2;
> invalid_flag = 0x1 << ((sizeof (int) * CHAR_BIT) - invalid_flag);
> diff --git a/misc/tst-preadvwritev2.c b/misc/tst-preadvwritev2.c
> index be22802dbe..cb58cbe41e 100644
> --- a/misc/tst-preadvwritev2.c
> +++ b/misc/tst-preadvwritev2.c
> @@ -30,6 +30,8 @@ do_test (void)
> {
> do_test_with_invalid_flags ();
> do_test_without_offset ();
> + do_test_with_invalid_fd ();
> + do_test_with_invalid_iov ();
>
> return do_test_with_offset (0);
> }
> diff --git a/misc/tst-preadvwritev64v2.c b/misc/tst-preadvwritev64v2.c
> index 8d3cc32b28..6a9de54c78 100644
> --- a/misc/tst-preadvwritev64v2.c
> +++ b/misc/tst-preadvwritev64v2.c
> @@ -32,6 +32,8 @@ do_test (void)
> {
> do_test_with_invalid_flags ();
> do_test_without_offset ();
> + do_test_with_invalid_fd ();
> + do_test_with_invalid_iov ();
>
> return do_test_with_offset (0);
> }
> diff --git a/sysdeps/unix/sysv/linux/preadv2.c b/sysdeps/unix/sysv/linux/preadv2.c
> index c8bf0764ef..bb08cbc5fd 100644
> --- a/sysdeps/unix/sysv/linux/preadv2.c
> +++ b/sysdeps/unix/sysv/linux/preadv2.c
> @@ -32,7 +32,7 @@ preadv2 (int fd, const struct iovec *vector, int count, off_t offset,
> # ifdef __NR_preadv2
> ssize_t result = SYSCALL_CANCEL (preadv2, fd, vector, count,
> LO_HI_LONG (offset), flags);
> - if (result >= 0)
> + if (result >= 0 || errno != ENOSYS)
> return result;
> # endif
> /* Trying to emulate the preadv2 syscall flags is troublesome:
> diff --git a/sysdeps/unix/sysv/linux/preadv64v2.c b/sysdeps/unix/sysv/linux/preadv64v2.c
> index d7400a0252..b72a047347 100644
> --- a/sysdeps/unix/sysv/linux/preadv64v2.c
> +++ b/sysdeps/unix/sysv/linux/preadv64v2.c
> @@ -30,7 +30,7 @@ preadv64v2 (int fd, const struct iovec *vector, int count, off64_t offset,
> #ifdef __NR_preadv64v2
> ssize_t result = SYSCALL_CANCEL (preadv64v2, fd, vector, count,
> LO_HI_LONG (offset), flags);
> - if (result >= 0)
> + if (result >= 0 || errno != ENOSYS)
> return result;
> #endif
> /* Trying to emulate the preadv2 syscall flags is troublesome:
> diff --git a/sysdeps/unix/sysv/linux/pwritev2.c b/sysdeps/unix/sysv/linux/pwritev2.c
> index 29c2264c8f..26333ebd43 100644
> --- a/sysdeps/unix/sysv/linux/pwritev2.c
> +++ b/sysdeps/unix/sysv/linux/pwritev2.c
> @@ -28,7 +28,7 @@ pwritev2 (int fd, const struct iovec *vector, int count, off_t offset,
> # ifdef __NR_pwritev2
> ssize_t result = SYSCALL_CANCEL (pwritev2, fd, vector, count,
> LO_HI_LONG (offset), flags);
> - if (result >= 0)
> + if (result >= 0 || errno != ENOSYS)
> return result;
> # endif
> /* Trying to emulate the pwritev2 syscall flags is troublesome:
> diff --git a/sysdeps/unix/sysv/linux/pwritev64v2.c b/sysdeps/unix/sysv/linux/pwritev64v2.c
> index 42da321149..17ea905aa6 100644
> --- a/sysdeps/unix/sysv/linux/pwritev64v2.c
> +++ b/sysdeps/unix/sysv/linux/pwritev64v2.c
> @@ -30,7 +30,7 @@ pwritev64v2 (int fd, const struct iovec *vector, int count, off64_t offset,
> #ifdef __NR_pwritev64v2
> ssize_t result = SYSCALL_CANCEL (pwritev64v2, fd, vector, count,
> LO_HI_LONG (offset), flags);
> - if (result >= 0)
> + if (result >= 0 || errno != ENOSYS)
> return result;
> #endif
> /* Trying to emulate the pwritev2 syscall flags is troublesome:
>