[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/pkeys/.gitignore |   1 +
 testcases/kernel/syscalls/pkeys/pkey02.c   | 134 +++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pkeys/pkey02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 6ea991f12..fddf071f9 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -722,6 +722,7 @@ mprotect03 mprotect03
 mprotect04 mprotect04
 
 pkey01 pkey01
+pkey02 pkey02
 
 mq_notify01 mq_notify01
 mq_notify02 mq_notify02
diff --git a/testcases/kernel/syscalls/pkeys/.gitignore b/testcases/kernel/syscalls/pkeys/.gitignore
index 6fd5addb8..2f9ea0a1f 100644
--- a/testcases/kernel/syscalls/pkeys/.gitignore
+++ b/testcases/kernel/syscalls/pkeys/.gitignore
@@ -1 +1,2 @@
 /pkey01
+/pkey02
diff --git a/testcases/kernel/syscalls/pkeys/pkey02.c b/testcases/kernel/syscalls/pkeys/pkey02.c
new file mode 100644
index 000000000..ed6ea3c9d
--- /dev/null
+++ b/testcases/kernel/syscalls/pkeys/pkey02.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Red Hat, Inc.
+ *
+ * Test for Memory Protection Keys on different memory area.
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/syscall.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+
+#include "pkey.h"
+
+#define TMP_DIR "tmp_pkey"
+#define TEST_FILE TMP_DIR"/testfile"
+#define STR "abcdefghijklmnopqrstuvwxyz12345\n"
+
+static int psize;
+
+static struct tcase {
+	unsigned long flags;
+	unsigned long access_rights;
+} tcases[] = {
+	{0, 0x0},
+	{0, PKEY_DISABLE_ACCESS},
+	{0, PKEY_DISABLE_WRITE},
+};
+
+static void setup(void)
+{
+	int i, fd;
+
+	psize = getpagesize();
+	SAFE_MKDIR(TMP_DIR, 0664);
+	SAFE_MOUNT(TMP_DIR, TMP_DIR, "tmpfs", 0, NULL);
+
+	check_pkey_support();
+
+	fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
+	for (i = 0; i < 128; i++)
+		SAFE_WRITE(1, fd, STR, strlen(STR));
+
+	SAFE_CLOSE(fd);
+}
+
+static void cleanup(void)
+{
+	SAFE_UMOUNT(TMP_DIR);
+	SAFE_RMDIR(TMP_DIR);
+}
+
+static struct mmap_param {
+	int prot;
+	int flags;
+	int fd;
+} mmap_params[] = {
+	{PROT_READ,  MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_READ,  MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_READ, MAP_PRIVATE, 0},
+	{PROT_READ, MAP_SHARED, 0},
+	{PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_WRITE, MAP_PRIVATE, 0},
+	{PROT_WRITE, MAP_SHARED, 0},
+	{PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_EXEC, MAP_PRIVATE, 0},
+	{PROT_EXEC, MAP_SHARED, 0},
+	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_READ | PROT_WRITE, MAP_PRIVATE, 0},
+	{PROT_READ | PROT_WRITE, MAP_SHARED, 0},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE, 0},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, 0},
+};
+
+static void pkey_tests(int pkey, int prot, int flags, int fd)
+{
+	char *buffer;
+
+	if (fd == 0) {
+		fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
+	}
+
+	buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0);
+
+	if (pkey_mprotect(buffer, psize, prot, pkey) == -1)
+		tst_brk(TBROK, "pkey_mprotect failed");
+
+	tst_res(TPASS, "apply pkey to the buffer area success");
+
+	if (fd > 0) {
+		SAFE_CLOSE(fd);
+	}
+
+	SAFE_MUNMAP(buffer, psize);
+}
+
+static void verify_pkey(unsigned int i)
+{
+	int pkey;
+	long unsigned int j;
+
+	struct tcase *tc = &tcases[i];
+	struct mmap_param *mpa;
+
+	for (j = 0; j < ARRAY_SIZE(mmap_params); j++) {
+		mpa = &mmap_params[j];
+
+		pkey = pkey_alloc(tc->flags, tc->access_rights);
+		if (pkey == -1)
+			tst_brk(TBROK, "pkey_alloc failed");
+
+		pkey_tests(pkey, mpa->prot, mpa->flags, mpa->fd);
+
+		if (pkey_free(pkey) == -1)
+			tst_brk(TBROK, "pkey_free failed");
+	}
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.forks_child = 1,
+	.test = verify_pkey,
+	.setup = setup,
+	.cleanup = cleanup,
+};
-- 
2.20.1

[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[Jan Stancek]

----- Original Message -----
> Signed-off-by: Li Wang <liwang@redhat.com>
> +
> +static void pkey_tests(int pkey, int prot, int flags, int fd)
> +{
> +	char *buffer;
> +
> +	if (fd == 0) {
> +		fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
> +	}
> +
> +	buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0);
> +
> +	if (pkey_mprotect(buffer, psize, prot, pkey) == -1)
> +		tst_brk(TBROK, "pkey_mprotect failed");
> +
> +	tst_res(TPASS, "apply pkey to the buffer area success");
> +
> +	if (fd > 0) {
> +		SAFE_CLOSE(fd);
> +	}
> +
> +	SAFE_MUNMAP(buffer, psize);
> +}
> +

Hi,

pkey02 doesn't try to read/write as pkey01, but otherwise two tests look
very similar.

Could we try to read/write here as well for all combinations of map flags?
Then pkey01 could be dropped since pkey02 would cover more than just 1 combination.
Or is there a different reason behind separate tests, that I'm missing?

Regards,
Jan

[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[Li Wang]

On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote:

>
>
> ----- Original Message -----
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > +
> > +static void pkey_tests(int pkey, int prot, int flags, int fd)
> > +{
> > +     char *buffer;
> > +
> > +     if (fd == 0) {
> > +             fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
> > +     }
> > +
> > +     buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0);
> > +
> > +     if (pkey_mprotect(buffer, psize, prot, pkey) == -1)
> > +             tst_brk(TBROK, "pkey_mprotect failed");
> > +
> > +     tst_res(TPASS, "apply pkey to the buffer area success");
> > +
> > +     if (fd > 0) {
> > +             SAFE_CLOSE(fd);
> > +     }
> > +
> > +     SAFE_MUNMAP(buffer, psize);
> > +}
> > +
>
> Hi,
>
> pkey02 doesn't try to read/write as pkey01, but otherwise two tests look
> very similar.
>
> Could we try to read/write here as well for all combinations of map flags?
> Then pkey01 could be dropped since pkey02 would cover more than just 1
> combination.
> Or is there a different reason behind separate tests, that I'm missing?
>
>
The pkey02 is a follow new test idea(test more types of memory) after I
completed pkey01.

Yes, the diffenrence bettwen them is pkey02 cover more map flags but not do
R/W verification. That's because I'm hoping to add {0, 0x0} to the test
which does not trigger SIGSEGV in pkey02, it seems a bit tricky to
distinguish the SIGSEGV is come from 0x0(if bug there)
or PKEY_DISABLE_ACCESS progress.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190624/4bcdc3e7/attachment.html>

[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[Li Wang]

On Mon, Jun 24, 2019 at 3:27 PM Li Wang <liwang@redhat.com> wrote:

>
>
> On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote:
>
>>
>>
>> ----- Original Message -----
>> > Signed-off-by: Li Wang <liwang@redhat.com>
>> > +
>> > +static void pkey_tests(int pkey, int prot, int flags, int fd)
>> > +{
>> > +     char *buffer;
>> > +
>> > +     if (fd == 0) {
>> > +             fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
>> > +     }
>> > +
>> > +     buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0);
>> > +
>> > +     if (pkey_mprotect(buffer, psize, prot, pkey) == -1)
>> > +             tst_brk(TBROK, "pkey_mprotect failed");
>> > +
>> > +     tst_res(TPASS, "apply pkey to the buffer area success");
>> > +
>> > +     if (fd > 0) {
>> > +             SAFE_CLOSE(fd);
>> > +     }
>> > +
>> > +     SAFE_MUNMAP(buffer, psize);
>> > +}
>> > +
>>
>> Hi,
>>
>> pkey02 doesn't try to read/write as pkey01, but otherwise two tests look
>> very similar.
>>
>> Could we try to read/write here as well for all combinations of map flags?
>> Then pkey01 could be dropped since pkey02 would cover more than just 1
>> combination.
>> Or is there a different reason behind separate tests, that I'm missing?
>>
>>
> The pkey02 is a follow new test idea(test more types of memory) after I
> completed pkey01.
>
> Yes, the diffenrence bettwen them is pkey02 cover more map flags but not
> do R/W verification. That's because I'm hoping to add {0, 0x0} to the test
> which does not trigger SIGSEGV in pkey02, it seems a bit tricky to
> distinguish the SIGSEGV is come from 0x0(if bug there)
> or PKEY_DISABLE_ACCESS progress.
>
>
Well, a smple way I can find is to remove the {0, 0x0} from struct tcase,
and test the access right 0x0 in lastly.

So, what do you think about the new key02(merge key01 and old key02) in
this attatchment?

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190624/8f076211/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pkey02_new.c
Type: text/x-csrc
Size: 4325 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190624/8f076211/attachment-0001.c>

[LTP] [PATCH v2 3/3] pkey: add pkey02 test

[Jan Stancek]

----- Original Message -----
> On Mon, Jun 24, 2019 at 3:27 PM Li Wang <liwang@redhat.com> wrote:
> 
> >
> >
> > On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote:
> >
> >>
> >>
> >> ----- Original Message -----
> >> > Signed-off-by: Li Wang <liwang@redhat.com>
> >> > +
> >> > +static void pkey_tests(int pkey, int prot, int flags, int fd)
> >> > +{
> >> > +     char *buffer;
> >> > +
> >> > +     if (fd == 0) {
> >> > +             fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
> >> > +     }
> >> > +
> >> > +     buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0);
> >> > +
> >> > +     if (pkey_mprotect(buffer, psize, prot, pkey) == -1)
> >> > +             tst_brk(TBROK, "pkey_mprotect failed");
> >> > +
> >> > +     tst_res(TPASS, "apply pkey to the buffer area success");
> >> > +
> >> > +     if (fd > 0) {
> >> > +             SAFE_CLOSE(fd);
> >> > +     }
> >> > +
> >> > +     SAFE_MUNMAP(buffer, psize);
> >> > +}
> >> > +
> >>
> >> Hi,
> >>
> >> pkey02 doesn't try to read/write as pkey01, but otherwise two tests look
> >> very similar.
> >>
> >> Could we try to read/write here as well for all combinations of map flags?
> >> Then pkey01 could be dropped since pkey02 would cover more than just 1
> >> combination.
> >> Or is there a different reason behind separate tests, that I'm missing?
> >>
> >>
> > The pkey02 is a follow new test idea(test more types of memory) after I
> > completed pkey01.
> >
> > Yes, the diffenrence bettwen them is pkey02 cover more map flags but not
> > do R/W verification. That's because I'm hoping to add {0, 0x0} to the test
> > which does not trigger SIGSEGV in pkey02, it seems a bit tricky to
> > distinguish the SIGSEGV is come from 0x0(if bug there)
> > or PKEY_DISABLE_ACCESS progress.
> >
> >
> Well, a smple way I can find is to remove the {0, 0x0} from struct tcase,
> and test the access right 0x0 in lastly.

You could also keep {0, 0x0}, but extend struct with "int fault_expected",
and set it to 0/1 depending on flags.

> 
> So, what do you think about the new key02(merge key01 and old key02) in
> this attatchment?

That works too. Some comments below:

	if (fd > 0) {
		SAFE_CLOSE(fd);
	}
0 is valid fd, brackets are not needed.

All tst_* functions that report failures should also have TERRNO.

tst_res(TFAIL, "Child unexpectedly ended") could print also exit code.

Regards,
Jan