Skip to contentSkip to conversation

pkey: add pkey02 test

PATCH New Li Wang 2 participants 5 messages Jun 21, 2019 – Jun 24, 2019

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH v2 3/3 pkey: add pkey02 test

---
 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
runtest/syscalls +1 -0
index 6ea991f12..fddf071f9 100644
@@ -722,6 +722,7 @@ mprotect03 mprotect03
 mprotect04 mprotect04
 
 pkey01 pkey01
+pkey02 pkey02
 
 mq_notify01 mq_notify01
 mq_notify02 mq_notify02
testcases/kernel/syscalls/pkeys/.gitignore +1 -0
index 6fd5addb8..2f9ea0a1f 100644
@@ -1 +1,2 @@
 /pkey01
+/pkey02
testcases/kernel/syscalls/pkeys/pkey02.c new file +134 -0

134 lines not on this page · raw message

Signature · 1 line
2.20.1
#1 lore raw

PATCH v2 3/3 pkey: add pkey02 test

----- Original Message -----

Quoted 14 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +13 -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");

testcases/kernel/syscalls/pkeys/pkey02.c line 105

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +10 -0
+
+	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

author
#2 lore raw

PATCH v2 3/3 pkey: add pkey02 test

Quoted 30 lines · Jan Stancek

----- Original Message -----

Quoted 14 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +13 -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");

testcases/kernel/syscalls/pkeys/pkey02.c line 105

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +10 -0
+
+     tst_res(TPASS, "apply pkey to the buffer area success");
+
+     if (fd > 0) {
+             SAFE_CLOSE(fd);
+     }
+
+     SAFE_MUNMAP(buffer, psize);
+}
+

Hi,

Quoted 9 lines

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.

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190624/4bcdc3e7/attachment.html>
author
#3 lore raw

PATCH v2 3/3 pkey: add pkey02 test

Quoted 43 lines · Li Wang
Quoted 30 lines · Jan Stancek

----- Original Message -----

Quoted 14 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +13 -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");

testcases/kernel/syscalls/pkeys/pkey02.c line 105

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +10 -0
+
+     tst_res(TPASS, "apply pkey to the buffer area success");
+
+     if (fd > 0) {
+             SAFE_CLOSE(fd);
+     }
+
+     SAFE_MUNMAP(buffer, psize);
+}
+

Hi,

Quoted 9 lines

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?

Quoted 10 lines

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?

Signature · 12 lines
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>
#4 lore raw

PATCH v2 3/3 pkey: add pkey02 test

----- Original Message -----

Quoted 55 lines · Li Wang
Quoted 44 lines · Jan Stancek
Quoted 31 lines

----- Original Message -----

Quoted 14 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +13 -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");

testcases/kernel/syscalls/pkeys/pkey02.c line 105

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey02.c new file +10 -0
+
+     tst_res(TPASS, "apply pkey to the buffer area success");
+
+     if (fd > 0) {
+             SAFE_CLOSE(fd);
+     }
+
+     SAFE_MUNMAP(buffer, psize);
+}
+

Hi,

Quoted 9 lines

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?

Quoted 10 lines

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.

Quoted 2 lines

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.

Quoted 3 lines

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