[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.

Signed-off-by: Li Wang <liwang@redhat.com>
---

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

----- Original Message -----
> 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.
> 
> Signed-off-by: Li Wang <liwang@redhat.com>

for series:
Acked-by: Jan Stancek <jstancek@redhat.com>

Hi!
Looks good, some minor comments below.

> 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.
> 
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
> 
> 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
> 
> diff --git a/configure.ac b/configure.ac
> index 65fc2a232..5a9b74b0a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -74,6 +74,7 @@ AC_CHECK_FUNCS([ \
>      pidfd_send_signal \
>      preadv \
>      preadv2 \
> +    pkey_mprotect \
>      profil \
>      pwritev \
>      pwritev2 \
> diff --git a/runtest/syscalls b/runtest/syscalls
> index c6a064481..6ea991f12 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -721,6 +721,8 @@ mprotect02 mprotect02
>  mprotect03 mprotect03
>  mprotect04 mprotect04
>  
> +pkey01 pkey01
> +
>  mq_notify01 mq_notify01
>  mq_notify02 mq_notify02
>  mq_open01 mq_open01
> diff --git a/testcases/kernel/syscalls/pkeys/.gitignore b/testcases/kernel/syscalls/pkeys/.gitignore
> new file mode 100644
> index 000000000..6fd5addb8
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pkeys/.gitignore
> @@ -0,0 +1 @@
> +/pkey01
> diff --git a/testcases/kernel/syscalls/pkeys/Makefile b/testcases/kernel/syscalls/pkeys/Makefile
> new file mode 100644
> index 000000000..9ee2c2ea5
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pkeys/Makefile
> @@ -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
> diff --git a/testcases/kernel/syscalls/pkeys/pkey.h b/testcases/kernel/syscalls/pkeys/pkey.h
> new file mode 100644
> index 000000000..d623244eb
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pkeys/pkey.h
> @@ -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 */
> diff --git a/testcases/kernel/syscalls/pkeys/pkey01.c b/testcases/kernel/syscalls/pkeys/pkey01.c
> new file mode 100644
> index 000000000..3b1e60ce6
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pkeys/pkey01.c
> @@ -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);
> +	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.

> +}
> +
> +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)
> +		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.

> +		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",
> +				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.

> +	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.

> +		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,
> +};
> -- 
> 2.20.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

On Tue, Jul 9, 2019 at 10:24 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> 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