* [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x
@ 2016-04-21 22:22 Jan Stancek
2016-04-25 15:19 ` Cyril Hrubis
0 siblings, 1 reply; 3+ messages in thread
From: Jan Stancek @ 2016-04-21 22:22 UTC (permalink / raw)
To: ltp
EFAULT testcase is behaving differently on s390, mainly because
access_ok() always returns true, so rw_copy_check_uvector() can't
detect that something is wrong with iovec we pass in.
1. access_ok() on s390/x always return true [1], so we don't
fail at rw_copy_check_uvector() as we do on other arches
preadv
vfs_readv
do_readv_writev
rw_copy_check_uvector
access_ok
2. With access_ok() ineffective, do_generic_file_read() returns
failure only if nothing could be read. So we pass just one bad
iovec to trigger EFAULT on s390.
preadv
vfs_readv
do_readv_writev
xfs_file_read_iter
generic_file_read_iter
do_generic_file_read
written ? written : error;
3. To avoid inode size check in do_generic_file_read() that would
prematurely end this function, we truncate new file to size of 1 page.
[1] d12a2970385c "s390/uaccess: remove pointless access_ok() checks"
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/syscalls/preadv/preadv02.c | 6 +++++-
testcases/kernel/syscalls/pwritev/pwritev02.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/testcases/kernel/syscalls/preadv/preadv02.c b/testcases/kernel/syscalls/preadv/preadv02.c
index ae9fa6e46e2a..3076b8c1df76 100644
--- a/testcases/kernel/syscalls/preadv/preadv02.c
+++ b/testcases/kernel/syscalls/preadv/preadv02.c
@@ -39,6 +39,7 @@
*/
#include <sys/uio.h>
+#include <unistd.h>
#include "tst_test.h"
#include "preadv.h"
@@ -57,7 +58,9 @@ static struct iovec rd_iovec1[] = {
};
static struct iovec rd_iovec2[] = {
+#if !defined(__s390__) && !defined(__s390x__)
{buf, CHUNK},
+#endif
{(char *)-1, CHUNK},
};
@@ -71,7 +74,7 @@ static struct tcase {
{&fd1, rd_iovec1, 1, 0, EINVAL},
{&fd1, rd_iovec2, -1, 0, EINVAL},
{&fd1, rd_iovec2, 1, -1, EINVAL},
- {&fd1, rd_iovec2, 2, 0, EFAULT},
+ {&fd1, rd_iovec2, ARRAY_SIZE(rd_iovec2), 0, EFAULT},
{&fd3, rd_iovec2, 1, 0, EBADF},
{&fd2, rd_iovec2, 1, 0, EBADF},
{&fd4, rd_iovec2, 1, 0, EISDIR},
@@ -101,6 +104,7 @@ static void verify_preadv(unsigned int n)
static void setup(void)
{
fd1 = SAFE_OPEN("file1", O_RDWR | O_CREAT, 0644);
+ SAFE_FTRUNCATE(fd1, getpagesize());
fd2 = SAFE_OPEN("file2", O_WRONLY | O_CREAT, 0644);
fd4 = SAFE_OPEN(".", O_RDONLY);
SAFE_PIPE(fd5);
diff --git a/testcases/kernel/syscalls/pwritev/pwritev02.c b/testcases/kernel/syscalls/pwritev/pwritev02.c
index d7ca1055644c..97f2cf0d5abc 100644
--- a/testcases/kernel/syscalls/pwritev/pwritev02.c
+++ b/testcases/kernel/syscalls/pwritev/pwritev02.c
@@ -37,6 +37,7 @@
*/
#include <sys/uio.h>
+#include <unistd.h>
#include "tst_test.h"
#include "pwritev.h"
@@ -54,7 +55,9 @@ static struct iovec wr_iovec1[] = {
};
static struct iovec wr_iovec2[] = {
+#if !defined(__s390__) && !defined(__s390x__)
{buf, CHUNK},
+#endif
{(char *)-1, CHUNK},
};
@@ -68,7 +71,7 @@ static struct tcase {
{&fd1, wr_iovec1, 1, 0, EINVAL},
{&fd1, wr_iovec2, -1, 0, EINVAL},
{&fd1, wr_iovec2, 1, -1, EINVAL},
- {&fd1, wr_iovec2, 2, 0, EFAULT},
+ {&fd1, wr_iovec2, ARRAY_SIZE(wr_iovec2), 0, EFAULT},
{&fd3, wr_iovec2, 1, 0, EBADF},
{&fd2, wr_iovec2, 1, 0, EBADF},
{&fd4[1], wr_iovec2, 1, 0, ESPIPE}
@@ -96,6 +99,7 @@ static void verify_pwritev(unsigned int n)
static void setup(void)
{
fd1 = SAFE_OPEN("file", O_RDWR | O_CREAT, 0644);
+ SAFE_FTRUNCATE(fd1, getpagesize());
fd2 = SAFE_OPEN("file", O_RDONLY | O_CREAT, 0644);
SAFE_PIPE(fd4);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x
2016-04-21 22:22 [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x Jan Stancek
@ 2016-04-25 15:19 ` Cyril Hrubis
2016-04-25 16:38 ` Jan Stancek
0 siblings, 1 reply; 3+ messages in thread
From: Cyril Hrubis @ 2016-04-25 15:19 UTC (permalink / raw)
To: ltp
Hi!
> static struct iovec rd_iovec2[] = {
> +#if !defined(__s390__) && !defined(__s390x__)
> {buf, CHUNK},
> +#endif
> {(char *)-1, CHUNK},
> };
What about splitting it as:
--- a/testcases/kernel/syscalls/preadv/preadv02.c
+++ b/testcases/kernel/syscalls/preadv/preadv02.c
@@ -58,6 +58,9 @@ static struct iovec rd_iovec1[] = {
static struct iovec rd_iovec2[] = {
{buf, CHUNK},
+};
+
+static struct iovec rd_iovec3[] = {
{(char *)-1, CHUNK},
};
@@ -71,7 +74,7 @@ static struct tcase {
{&fd1, rd_iovec1, 1, 0, EINVAL},
{&fd1, rd_iovec2, -1, 0, EINVAL},
{&fd1, rd_iovec2, 1, -1, EINVAL},
- {&fd1, rd_iovec2, 2, 0, EFAULT},
+ {&fd1, rd_iovec3, 1, 0, EFAULT},
{&fd3, rd_iovec2, 1, 0, EBADF},
{&fd2, rd_iovec2, 1, 0, EBADF},
{&fd4, rd_iovec2, 1, 0, EISDIR},
instead of adding #ifdefs?
That should work fine as well. Or do you want to reatain the part that tests
that we get EFAULT for the second part of the iovec as well?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 3+ messages in thread
* [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x
2016-04-25 15:19 ` Cyril Hrubis
@ 2016-04-25 16:38 ` Jan Stancek
0 siblings, 0 replies; 3+ messages in thread
From: Jan Stancek @ 2016-04-25 16:38 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Monday, 25 April, 2016 5:19:05 PM
> Subject: Re: [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x
>
> Hi!
> > static struct iovec rd_iovec2[] = {
> > +#if !defined(__s390__) && !defined(__s390x__)
> > {buf, CHUNK},
> > +#endif
> > {(char *)-1, CHUNK},
> > };
>
> What about splitting it as:
>
> --- a/testcases/kernel/syscalls/preadv/preadv02.c
> +++ b/testcases/kernel/syscalls/preadv/preadv02.c
> @@ -58,6 +58,9 @@ static struct iovec rd_iovec1[] = {
>
> static struct iovec rd_iovec2[] = {
> {buf, CHUNK},
> +};
> +
> +static struct iovec rd_iovec3[] = {
> {(char *)-1, CHUNK},
> };
>
> @@ -71,7 +74,7 @@ static struct tcase {
> {&fd1, rd_iovec1, 1, 0, EINVAL},
> {&fd1, rd_iovec2, -1, 0, EINVAL},
> {&fd1, rd_iovec2, 1, -1, EINVAL},
> - {&fd1, rd_iovec2, 2, 0, EFAULT},
> + {&fd1, rd_iovec3, 1, 0, EFAULT},
> {&fd3, rd_iovec2, 1, 0, EBADF},
> {&fd2, rd_iovec2, 1, 0, EBADF},
> {&fd4, rd_iovec2, 1, 0, EISDIR},
>
> instead of adding #ifdefs?
>
> That should work fine as well. Or do you want to reatain the part that tests
> that we get EFAULT for the second part of the iovec as well?
No, I think we can split it as you suggested. I'll run a quick test and post v2.
Regards,
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-25 16:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-21 22:22 [LTP] [PATCH] preadv02/pwritev02: fix EFAULT testcase on s390/x Jan Stancek
2016-04-25 15:19 ` Cyril Hrubis
2016-04-25 16:38 ` Jan Stancek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.