Skip to contentSkip to conversation

pkey: add syscall numbers for pkey

PATCH New Li Wang 3 participants 5 messages Jul 1, 2019 – Jul 17, 2019

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH v5 1/2 pkey: add syscall numbers for pkey

---
 include/lapi/syscalls/aarch64.in   | 3 +++
 include/lapi/syscalls/arm.in       | 3 +++
 include/lapi/syscalls/i386.in      | 3 +++
 include/lapi/syscalls/ia64.in      | 3 +++
 include/lapi/syscalls/powerpc.in   | 3 +++
 include/lapi/syscalls/powerpc64.in | 3 +++
 include/lapi/syscalls/s390.in      | 3 +++
 include/lapi/syscalls/s390x.in     | 3 +++
 include/lapi/syscalls/sparc.in     | 3 +++
 include/lapi/syscalls/sparc64.in   | 3 +++
 include/lapi/syscalls/x86_64.in    | 3 +++
 11 files changed, 33 insertions(+)
include/lapi/syscalls/aarch64.in +3 -0
index ce6080894..7db6e281c 100644
@@ -265,5 +265,8 @@ mlock2 284
 copy_file_range 285
 preadv2 286
 pwritev2 287
+pkey_mprotect 288
+pkey_alloc 289
+pkey_free 290
 pidfd_send_signal 424
 _sysctl 1078
include/lapi/syscalls/arm.in +3 -0
index 33168889f..00e99f2b9 100644
@@ -349,5 +349,8 @@ mlock2 (__NR_SYSCALL_BASE+390)
 copy_file_range (__NR_SYSCALL_BASE+391)
 preadv2 (__NR_SYSCALL_BASE+392)
 pwritev2 (__NR_SYSCALL_BASE+393)
+pkey_mprotect (__NR_SYSCALL_BASE+394)
+pkey_alloc (__NR_SYSCALL_BASE+395)
+pkey_free (__NR_SYSCALL_BASE+396)
 statx (__NR_SYSCALL_BASE+397)
 pidfd_send_signal (__NR_SYSCALL_BASE+424)
include/lapi/syscalls/i386.in +3 -0
index 3355b9127..02f3955ba 100644
@@ -347,5 +347,8 @@ mlock2 376
 copy_file_range 377
 preadv2 378
 pwritev2 379
+pkey_mprotect 380
+pkey_alloc 381
+pkey_free 382
 statx 383
 pidfd_send_signal 424
include/lapi/syscalls/ia64.in +3 -0
index 9ac81e91e..cf9f73e85 100644
@@ -305,4 +305,7 @@ mlock2 1346
 copy_file_range 1347
 preadv2 1348
 pwritev2 1349
+pkey_mprotect 1354
+pkey_alloc 1355
+pkey_free 1356
 pidfd_send_signal 1448
include/lapi/syscalls/powerpc.in +3 -0
index 488d6dfa9..660165d7a 100644
@@ -356,3 +356,6 @@ preadv2 380
 pwritev2 381
 statx 383
 pidfd_send_signal 424
+pkey_mprotect 386
+pkey_alloc 384
+pkey_free 385
include/lapi/syscalls/powerpc64.in +3 -0
index 488d6dfa9..660165d7a 100644
@@ -356,3 +356,6 @@ preadv2 380
 pwritev2 381
 statx 383
 pidfd_send_signal 424
+pkey_mprotect 386
+pkey_alloc 384
+pkey_free 385
include/lapi/syscalls/s390.in +3 -0
index 69b49eac0..c304ef4b7 100644
@@ -338,4 +338,7 @@ mlock2 374
 copy_file_range 375
 preadv2 376
 pwritev2 377
+pkey_mprotect 384
+pkey_alloc 385
+pkey_free 386
 pidfd_send_signal 424
include/lapi/syscalls/s390x.in +3 -0
index 685c16f68..7d632d1dc 100644
@@ -337,4 +337,7 @@ mlock2 374
 copy_file_range 375
 preadv2 376
 pwritev2 377
+pkey_mprotect 384
+pkey_alloc 385
+pkey_free 386
 pidfd_send_signal 424
include/lapi/syscalls/sparc.in +3 -0
index e74b48170..ab7204663 100644
@@ -343,4 +343,7 @@ mlock2 356
 copy_file_range 357
 preadv2 358
 pwritev2 359
+pkey_mprotect 362
+pkey_alloc 363
+pkey_free 364
 pidfd_send_signal 424
include/lapi/syscalls/sparc64.in +3 -0
index 577aa9b35..d17dce5cd 100644
@@ -319,4 +319,7 @@ mlock2 356
 copy_file_range 357
 preadv2 358
 pwritev2 359
+pkey_mprotect 362
+pkey_alloc 363
+pkey_free 364
 pidfd_send_signal 424
include/lapi/syscalls/x86_64.in +3 -0
index 99d387277..fdb414c10 100644
@@ -314,5 +314,8 @@ mlock2 325
 copy_file_range 326
 preadv2 327
 pwritev2 328
+pkey_mprotect 329
+pkey_alloc 330
+pkey_free 331
 statx 332
 pidfd_send_signal 424
Signature · 1 line
2.20.1
author
#1 lore raw

PATCH v5 2/2 pkey: add test for memory protection keys

Memory Protection Keys for Userspace (PKU aka PKEYs) is a Skylake-SP server feature that provides a mechanism for enforcing page-based protections, but without requiring modification of the page tables when an application changes protection domains. It works by dedicating 4 previously ignored bits in each page table entry to a "protection key", giving 16 possible keys.

Basic method for PKEYs test:
1. test allocates a pkey(e.g. PKEY_DISABLE_ACCESS) via pkey_alloc()
2. pkey_mprotect() apply this pkey to a piece of memory buffer
3. check if the access right of buffer has been changed and take effect
4. remove the access right(pkey) from this buffer via pkey_mprotect()
5. check if buffer area can be read or write after removing pkey
6. pkey_free() releases the pkey after using it

Looping around this basic test on diffenrent types of memory.

---

Notes:
v4 --> v5
* use "/sys/kernel/mm/hugepages/" for huge page detection

 configure.ac                               |   1 +
 runtest/syscalls                           |   2 +
 testcases/kernel/syscalls/pkeys/.gitignore |   1 +
 testcases/kernel/syscalls/pkeys/Makefile   |   8 +
 testcases/kernel/syscalls/pkeys/pkey.h     |  50 ++++
 testcases/kernel/syscalls/pkeys/pkey01.c   | 263 +++++++++++++++++++++
 6 files changed, 325 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pkeys/.gitignore
 create mode 100644 testcases/kernel/syscalls/pkeys/Makefile
 create mode 100644 testcases/kernel/syscalls/pkeys/pkey.h
 create mode 100644 testcases/kernel/syscalls/pkeys/pkey01.c
configure.ac +1 -0
index 65fc2a232..5a9b74b0a 100644
@@ -74,6 +74,7 @@ AC_CHECK_FUNCS([ \
     pidfd_send_signal \
     preadv \
     preadv2 \
+    pkey_mprotect \
     profil \
     pwritev \
     pwritev2 \
runtest/syscalls +2 -0
index c6a064481..6ea991f12 100644
@@ -721,6 +721,8 @@ mprotect02 mprotect02
 mprotect03 mprotect03
 mprotect04 mprotect04
 
+pkey01 pkey01
+
 mq_notify01 mq_notify01
 mq_notify02 mq_notify02
 mq_open01 mq_open01
testcases/kernel/syscalls/pkeys/.gitignore new file +1 -0
new file mode 100644
index 000000000..6fd5addb8
@@ -0,0 +1 @@
+/pkey01
testcases/kernel/syscalls/pkeys/Makefile new file +8 -0
new file mode 100644
index 000000000..9ee2c2ea5
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2019 Red Hat, Inc.
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/kernel/syscalls/pkeys/pkey.h new file +50 -0

50 lines not on this page · raw message

testcases/kernel/syscalls/pkeys/pkey01.c new file +263 -0

263 lines not on this page · raw message

Signature · 1 line
2.20.1
#2 lore raw

PATCH v5 2/2 pkey: add test for memory protection keys

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

Quoted 15 lines

Memory Protection Keys for Userspace (PKU aka PKEYs) is a Skylake-SP server feature that provides a mechanism for enforcing page-based protections, but without requiring modification of the page tables when an application changes protection domains. It works by dedicating 4 previously ignored bits in each page table entry to a "protection key", giving 16 possible keys.

Basic method for PKEYs test:
1. test allocates a pkey(e.g. PKEY_DISABLE_ACCESS) via pkey_alloc()
2. pkey_mprotect() apply this pkey to a piece of memory buffer
3. check if the access right of buffer has been changed and take effect
4. remove the access right(pkey) from this buffer via pkey_mprotect()
5. check if buffer area can be read or write after removing pkey
6. pkey_free() releases the pkey after using it

Quoted 3 lines

Looping around this basic test on diffenrent types of memory.

for series:

#3 lore raw

PATCH v5 2/2 pkey: add test for memory protection keys

In reply to Li Wang

Hi!
Looks good, some minor comments below.

Quoted 223 lines

Memory Protection Keys for Userspace (PKU aka PKEYs) is a Skylake-SP server feature that provides a mechanism for enforcing page-based protections, but without requiring modification of the page tables when an application changes protection domains. It works by dedicating 4 previously ignored bits in each page table entry to a "protection key", giving 16 possible keys.

Basic method for PKEYs test:
1. test allocates a pkey(e.g. PKEY_DISABLE_ACCESS) via pkey_alloc()
2. pkey_mprotect() apply this pkey to a piece of memory buffer
3. check if the access right of buffer has been changed and take effect
4. remove the access right(pkey) from this buffer via pkey_mprotect()
5. check if buffer area can be read or write after removing pkey
6. pkey_free() releases the pkey after using it

Looping around this basic test on diffenrent types of memory.

---

Notes:
v4 --> v5
* use "/sys/kernel/mm/hugepages/" for huge page detection

 configure.ac                               |   1 +
 runtest/syscalls                           |   2 +
 testcases/kernel/syscalls/pkeys/.gitignore |   1 +
 testcases/kernel/syscalls/pkeys/Makefile   |   8 +
 testcases/kernel/syscalls/pkeys/pkey.h     |  50 ++++
 testcases/kernel/syscalls/pkeys/pkey01.c   | 263 +++++++++++++++++++++
 6 files changed, 325 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pkeys/.gitignore
 create mode 100644 testcases/kernel/syscalls/pkeys/Makefile
 create mode 100644 testcases/kernel/syscalls/pkeys/pkey.h
 create mode 100644 testcases/kernel/syscalls/pkeys/pkey01.c
configure.ac +1 -0
index 65fc2a232..5a9b74b0a 100644
@@ -74,6 +74,7 @@ AC_CHECK_FUNCS([ \
     pidfd_send_signal \
     preadv \
     preadv2 \
+    pkey_mprotect \
     profil \
     pwritev \
     pwritev2 \
runtest/syscalls +2 -0
index c6a064481..6ea991f12 100644
@@ -721,6 +721,8 @@ mprotect02 mprotect02
 mprotect03 mprotect03
 mprotect04 mprotect04
 
+pkey01 pkey01
+
 mq_notify01 mq_notify01
 mq_notify02 mq_notify02
 mq_open01 mq_open01
testcases/kernel/syscalls/pkeys/.gitignore new file +1 -0
new file mode 100644
index 000000000..6fd5addb8
@@ -0,0 +1 @@
+/pkey01
testcases/kernel/syscalls/pkeys/Makefile new file +8 -0
new file mode 100644
index 000000000..9ee2c2ea5
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2019 Red Hat, Inc.
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/kernel/syscalls/pkeys/pkey.h new file +50 -0
new file mode 100644
index 000000000..d623244eb
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Red Hat, Inc.
+ */
+
+#ifndef PKEYS_H
+#define PKEYS_H
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#ifndef PKEY_DISABLE_ACCESS
+# define PKEY_DISABLE_ACCESS 0x1
+# define PKEY_DISABLE_WRITE  0x2
+#endif
+
+#ifndef HAVE_PKEY_MPROTECT
+static inline int pkey_mprotect(void *addr, size_t len, int prot, int pkey)
+{
+	return tst_syscall(__NR_pkey_mprotect, addr, len, prot, pkey);
+}
+
+static inline int pkey_alloc(unsigned int flags, unsigned int access_rights)
+{
+	return tst_syscall(__NR_pkey_alloc, flags, access_rights);
+}
+
+static inline int pkey_free(int pkey)
+{
+	return tst_syscall(__NR_pkey_free, pkey);
+}
+#endif /* HAVE_PKEY_MPROTECT */
+
+static inline void check_pkey_support(void)
+{
+	int pkey = pkey_alloc(0, 0);
+
+	if (pkey == -1) {
+		if (errno == ENOSYS)
+			tst_brk(TCONF, "pkey_alloc is not implemented");
+		if (errno == EINVAL)
+			tst_brk(TCONF, "pku is not supported on this CPU");
+		if (errno == ENOSPC)
+			tst_brk(TCONF, "pkeys are not available for test");
+	}
+
+	pkey_free(pkey);
+}
+
+#endif /* PKEYS_H */
testcases/kernel/syscalls/pkeys/pkey01.c new file +79 -0
new file mode 100644
index 000000000..3b1e60ce6
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Red Hat, Inc.
+ *
+ * Memory Protection Keys for Userspace (PKU aka PKEYs) is a Skylake-SP
+ * server feature that provides a mechanism for enforcing page-based
+ * protections, but without requiring modification of the page tables
+ * when an application changes protection domains. It works by dedicating
+ * 4 previously ignored bits in each page table entry to a "protection key",
+ * giving 16 possible keys.
+ *
+ * Basic method for PKEYs testing:
+ *    1. test allocates a pkey(e.g. PKEY_DISABLE_ACCESS) via pkey_alloc()
+ *    2. pkey_mprotect() apply this pkey to a piece of memory(buffer)
+ *    3. check if access right of the buffer has been changed and take effect
+ *    4. remove the access right(pkey) from this buffer via pkey_mprotect()
+ *    5. check if buffer area can be read or write after removing pkey
+ *    6. pkey_free() releases the pkey after using it
+ *
+ * Looping around this basic test on diffenrent types of memory.
+ */
+
+#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"
+#define PATH_VM_NRHPS "/proc/sys/vm/nr_hugepages"
+
+static int size;
+static int no_hugepage;
+
+static const char * const save_restore[] = {
+	"?/proc/sys/vm/nr_hugepages",
+	NULL,
+};
+
+static struct tcase {
+	unsigned long flags;
+	unsigned long access_rights;
+	char *name;
+} tcases[] = {
+	{0, PKEY_DISABLE_ACCESS, "PKEY_DISABLE_ACCESS"},
+	{0, PKEY_DISABLE_WRITE, "PKEY_DISABLE_WRITE"},
+};
+
+static void setup(void)
+{
+	int i, fd;
+
+	if (access("/sys/kernel/mm/hugepages/", F_OK)) {
+		tst_res(TINFO, "Huge page is not supported");
+		size = getpagesize();
+		no_hugepage = 1;
+	} else {
+		int val;
+		SAFE_FILE_PRINTF(PATH_VM_NRHPS, "%d", 1);
+		SAFE_FILE_SCANF(PATH_VM_NRHPS, "%d", &val);
+		if (val != 1)
+			tst_brk(TBROK, "nr_hugepages = %d, but expect %d",
+					val, 1);
+		size = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
+	}
+
+	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);

testcases/kernel/syscalls/pkeys/pkey01.c line 89

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

No need for the RMDIR here.

Quoted 86 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +86 -0
+}
+
+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_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB, -1},
+	{PROT_READ,  MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB, -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_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB, -1},
+	{PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB, -1},
+
+	{PROT_EXEC,  MAP_ANONYMOUS | MAP_PRIVATE, -1},
+	{PROT_EXEC,  MAP_ANONYMOUS | MAP_SHARED, -1},
+	{PROT_EXEC,  MAP_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB, -1},
+	{PROT_EXEC,  MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB, -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_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB, -1},
+	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB, -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_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB, -1},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB, -1},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE, 0},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, 0},
+};
+
+static char *flag_to_str(int flags)
+{
+	switch (flags) {
+	case MAP_PRIVATE:
+		return "MAP_PRIVATE";
+	case MAP_SHARED:
+		return "MAP_SHARED";
+	case MAP_ANONYMOUS | MAP_PRIVATE:
+		return "MAP_ANONYMOUS|MAP_PRIVATE";
+	case MAP_ANONYMOUS | MAP_SHARED:
+		return "MAP_ANONYMOUS|MAP_SHARED";
+	case MAP_ANONYMOUS | MAP_PRIVATE | MAP_HUGETLB:
+		return "MAP_ANONYMOUS|MAP_PRIVATE|MAP_HUGETLB";
+	case MAP_ANONYMOUS | MAP_SHARED  | MAP_HUGETLB:
+		return "MAP_ANONYMOUS|MAP_SHARED|MAP_HUGETLB";
+	default:
+		return "UNKNOWN FLAGS";
+	}
+}
+
+static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
+{
+	pid_t pid;
+	char *buffer;
+	int pkey, status;
+	int fd = mpa->fd;
+
+	if (no_hugepage && (mpa->flags & MAP_HUGETLB)) {
+		tst_res(TINFO, "Skip test on (%s) buffer", flag_to_str(mpa->flags));
+		return;
+	}
+
+	if (fd == 0)
+		fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664);
+
+	buffer = SAFE_MMAP(NULL, size, mpa->prot, mpa->flags, fd, 0);
+
+	pkey = pkey_alloc(tc->flags, tc->access_rights);
+	if (pkey == -1)
+		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
+
+	tst_res(TINFO, "Set %s on (%s) buffer", tc->name, flag_to_str(mpa->flags));
+	if (pkey_mprotect(buffer, size, mpa->prot, pkey) == -1)

testcases/kernel/syscalls/pkeys/pkey01.c line 185

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +10 -0
+		tst_brk(TBROK | TERRNO, "pkey_mprotect failed");
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		struct rlimit r;
+
+		r.rlim_cur = 1;
+		r.rlim_max = 1;
+		/* Children crash are expected, avoid dumping corefile */
+		SAFE_SETRLIMIT(RLIMIT_CORE, &r);

So this should be merged after the tst_no_corefile() and rebased to make use of that.

Quoted 16 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +16 -0
+		switch (tc->access_rights) {
+			case PKEY_DISABLE_ACCESS:
+			tst_res(TFAIL | TERRNO,
+				"Read buffer success, buffer[0] = %d", *buffer);
+		break;
+			case PKEY_DISABLE_WRITE:
+			*buffer = 'a';
+		break;
+		}
+		exit(0);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+	if (WIFSIGNALED(status)) {
+		if (WTERMSIG(status) == SIGSEGV) {
+			tst_res(TPASS, "Child ended by %s as expected",

testcases/kernel/syscalls/pkeys/pkey01.c line 212

Quoted 10 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +10 -0
+				tst_strsig(SIGSEGV));
+		} else {
+			tst_res(TFAIL | TERRNO, "Child ended by %s unexpected" ,
+				tst_strsig(WTERMSIG(status)));
+		}
+	} else {
+		tst_res(TFAIL | TERRNO,
+			"Child unexpectedly ended with %d",
+			WEXITSTATUS(status));
+	}

We do have a helper function for printing the status content so this could be simplified to:

	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
		tst_res(TPASS, ...);
	else
		tst_res(TFAIL, "Child: %s", tst_strstatus(status));

Also there is no point in printing the errno (TERRNO flag) here.

testcases/kernel/syscalls/pkeys/pkey01.c line 219

Quoted 6 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +6 -0
+	tst_res(TINFO, "Remove %s from the buffer", tc->name);
+	if (pkey_mprotect(buffer, size, mpa->prot, 0x0) == -1)
+		tst_brk(TBROK | TERRNO, "pkey_mprotect failed");
+
+	switch (mpa->prot) {
+		case PROT_READ:

Indenting the case statements and not indeting the break looks really strange. At least the LKML coding style does not indent the case statements within the swith() statement.

Quoted 50 lines
testcases/kernel/syscalls/pkeys/pkey01.c new file +44 -0
+		tst_res(TPASS, "Read buffer success, buffer[0] = %d", *buffer);
+	break;
+		case PROT_WRITE:
+		*buffer = 'a';
+	break;
+		case PROT_READ | PROT_WRITE:
+		case PROT_READ | PROT_WRITE | PROT_EXEC:
+		*buffer = 'a';
+		tst_res(TPASS, "Read & Write buffer success, buffer[0] = %d", *buffer);
+	break;
+	}
+
+	if (fd >= 0)
+		SAFE_CLOSE(fd);
+
+	SAFE_MUNMAP(buffer, size);
+
+	if (pkey_free(pkey) == -1)
+		tst_brk(TBROK | TERRNO, "pkey_free failed");
+}
+
+static void verify_pkey(unsigned int i)
+{
+	long unsigned int j;
+	struct mmap_param *mpa;
+
+	struct tcase *tc = &tcases[i];
+
+	for (j = 0; j < ARRAY_SIZE(mmap_params); j++) {
+		mpa = &mmap_params[j];
+
+		pkey_test(tc, mpa);
+	}
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_root = 1,
+	.forks_child = 1,
+	.test = verify_pkey,
+	.setup = setup,
+	.cleanup = cleanup,
+	.save_restore = save_restore,
+};
Signature · 5 lines
2.20.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis
[email protected]
author
#4 lore raw

PATCH v5 2/2 pkey: add test for memory protection keys

Quoted 3 lines · Cyril Hrubis

Hi!
Looks good, some minor comments below.

Thanks! Pushed with the following changes:

1. add .needs_tmpdir = 1 in test struct
2. remove SAFE_MKDIR/SAFE_MOUNT from setup()
3. remove cleanup() function
4. take use of tst_no_corefile()
5. adjust the status content printing
6. adjust the code indent for switch case

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190717/e7d20549/attachment.htm>