[LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[Li Wang]

The pkey_test function now includes a code snippet to test execute
permissions, ensuring proper handling of execution rights in memory
protection keys.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/lapi/pkey.h                      |  4 ++
 testcases/kernel/syscalls/pkeys/pkey01.c | 50 ++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/include/lapi/pkey.h b/include/lapi/pkey.h
index 398e3be5f..eb9bf7fb4 100644
--- a/include/lapi/pkey.h
+++ b/include/lapi/pkey.h
@@ -17,6 +17,10 @@
 # define PKEY_DISABLE_WRITE  0x2
 #endif
 
+#ifndef PKEY_DISABLE_EXECUTE
+# define PKEY_DISABLE_EXECUTE 0x4
+#endif
+
 #ifndef HAVE_PKEY_MPROTECT
 inline int pkey_mprotect(void *addr, size_t len, int prot, int pkey)
 {
diff --git a/testcases/kernel/syscalls/pkeys/pkey01.c b/testcases/kernel/syscalls/pkeys/pkey01.c
index f1ecfec0b..70fdffe48 100644
--- a/testcases/kernel/syscalls/pkeys/pkey01.c
+++ b/testcases/kernel/syscalls/pkeys/pkey01.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2019 Red Hat, Inc.
+ * Copyright (c) 2019-2024 Red Hat, Inc.
  */
 
 /*\
@@ -41,6 +41,7 @@
 #define PATH_VM_NRHPS "/proc/sys/vm/nr_hugepages"
 
 static int size;
+static int execute_supported = 1;
 
 static struct tcase {
 	unsigned long flags;
@@ -49,14 +50,26 @@ static struct tcase {
 } tcases[] = {
 	{0, PKEY_DISABLE_ACCESS, "PKEY_DISABLE_ACCESS"},
 	{0, PKEY_DISABLE_WRITE, "PKEY_DISABLE_WRITE"},
+	{0, PKEY_DISABLE_EXECUTE, "PKEY_DISABLE_EXECUTE"},
 };
 
 static void setup(void)
 {
-	int i, fd;
+	int i, fd, pkey;
 
 	check_pkey_support();
 
+	pkey = pkey_alloc(0, PKEY_DISABLE_EXECUTE);
+	if (pkey == -1) {
+		if (errno == EINVAL) {
+			tst_res(TINFO, "PKEY_DISABLE_EXECUTE hasn't implement.");
+			execute_supported = 0;
+		} else {
+			tst_brk(TBROK | TERRNO, "pkey_alloc failed");
+		}
+	}
+	pkey_free(pkey);
+
 	if (tst_hugepages == test.hugepages.number)
 		size = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
 	else
@@ -130,12 +143,28 @@ static char *flag_to_str(int flags)
 	}
 }
 
+static size_t function_size(void (*func)(void))
+{
+	unsigned char *start = (unsigned char *)func;
+	unsigned char *end = start;
+
+	while (*end != 0xC3 && *end != 0xC2)
+		end++;
+
+	return (size_t)(end - start + 1);
+}
+
 static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 {
 	pid_t pid;
 	char *buffer;
 	int pkey, status;
 	int fd = mpa->fd;
+	size_t (*func)();
+	size_t func_size = -1;
+
+	if (!execute_supported && (tc->access_rights == PKEY_DISABLE_EXECUTE))
+		tst_brk(TCONF, "skip PKEY_DISABLE_EXECUTE test");
 
 	if (!tst_hugepages && (mpa->flags & MAP_HUGETLB)) {
 		tst_res(TINFO, "Skip test on (%s) buffer", flag_to_str(mpa->flags));
@@ -147,6 +176,11 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 
 	buffer = SAFE_MMAP(NULL, size, mpa->prot, mpa->flags, fd, 0);
 
+	if (mpa->prot == (PROT_READ | PROT_WRITE | PROT_EXEC)) {
+		func_size = function_size((void (*)(void))function_size);
+		memcpy(buffer, (void *)function_size, func_size);
+	}
+
 	pkey = pkey_alloc(tc->flags, tc->access_rights);
 	if (pkey == -1)
 		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
@@ -169,6 +203,10 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 			tst_res(TFAIL | TERRNO,
 				"Write buffer success, buffer[0] = %d", *buffer);
 		break;
+		case PKEY_DISABLE_EXECUTE:
+			func = (size_t (*)())buffer;;
+			tst_res(TFAIL | TERRNO, "Excute buffer result = %lu", func(func));
+		break;
 		}
 		exit(0);
 	}
@@ -193,10 +231,16 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 		tst_res(TPASS, "Write buffer success, buffer[0] = %d", *buffer);
 	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;
+	case PROT_READ | PROT_WRITE | PROT_EXEC:
+		func = (size_t (*)())buffer;;
+		if (func_size == func(func))
+			tst_res(TPASS, "Execute buffer success, result = %lu", func_size);
+		else
+			tst_res(TFAIL, "Execute buffer with unexpected result: %lu", func(func));
+	break;
 	}
 
 	if (fd >= 0)
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[Petr Vorel]

Hi Li,

> The pkey_test function now includes a code snippet to test execute
> permissions, ensuring proper handling of execution rights in memory
> protection keys.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM, thanks!

Few notes + tiny fixes below (which can be done before merge).

...
> +	pkey = pkey_alloc(0, PKEY_DISABLE_EXECUTE);
> +	if (pkey == -1) {
> +		if (errno == EINVAL) {
> +			tst_res(TINFO, "PKEY_DISABLE_EXECUTE hasn't implement.");
nit: maybe "PKEY_DISABLE_EXECUTE not implemented"

> +			execute_supported = 0;
> +		} else {
> +			tst_brk(TBROK | TERRNO, "pkey_alloc failed");
> +		}
> +	}
> +	pkey_free(pkey);
...

>  static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  {
>  	pid_t pid;
>  	char *buffer;
>  	int pkey, status;
>  	int fd = mpa->fd;
> +	size_t (*func)();
> +	size_t func_size = -1;
> +
> +	if (!execute_supported && (tc->access_rights == PKEY_DISABLE_EXECUTE))
> +		tst_brk(TCONF, "skip PKEY_DISABLE_EXECUTE test");
NOTE due to possible tst_brk() it'd be good to keep PKEY_DISABLE_EXECUTE as a
last test. Not sure if it's worth to add a comment (or replace with tst_res()
and return).

...
> +	if (mpa->prot == (PROT_READ | PROT_WRITE | PROT_EXEC)) {
> +		func_size = function_size((void (*)(void))function_size);
> +		memcpy(buffer, (void *)function_size, func_size);
> +	}
> +
>  	pkey = pkey_alloc(tc->flags, tc->access_rights);
>  	if (pkey == -1)
>  		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
> @@ -169,6 +203,10 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  			tst_res(TFAIL | TERRNO,
>  				"Write buffer success, buffer[0] = %d", *buffer);
>  		break;
> +		case PKEY_DISABLE_EXECUTE:
> +			func = (size_t (*)())buffer;;
nit: double ; (please remove the last one).
> +			tst_res(TFAIL | TERRNO, "Excute buffer result = %lu", func(func));
s/%lu/%zi/ (otherwise it introduces warnings on 32bit)
nit: s/Excute/Execute/

> +		break;
>  		}
>  		exit(0);
>  	}
> @@ -193,10 +231,16 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
>  		tst_res(TPASS, "Write buffer success, buffer[0] = %d", *buffer);
>  	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;
> +	case PROT_READ | PROT_WRITE | PROT_EXEC:
> +		func = (size_t (*)())buffer;;
> +		if (func_size == func(func))
> +			tst_res(TPASS, "Execute buffer success, result = %lu", func_size);
s/%lu/%zi/
> +		else
> +			tst_res(TFAIL, "Execute buffer with unexpected result: %lu", func(func));
s/%lu/%zi/

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[Li Wang]

Hi Petr,

Petr Vorel <pvorel@suse.cz> wrote:


> > +     if (!execute_supported && (tc->access_rights ==
> PKEY_DISABLE_EXECUTE))
> > +             tst_brk(TCONF, "skip PKEY_DISABLE_EXECUTE test");
> NOTE due to possible tst_brk() it'd be good to keep PKEY_DISABLE_EXECUTE
> as a
> last test. Not sure if it's worth to add a comment (or replace with
> tst_res()
> and return).
>

Or, we can use 'tst_res + return' which is similar to tst_hugepages missing
way.

The rest comments all look good.


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey01: Adding test for PKEY_DISABLE_EXECUTE

[Li Wang]

Pushed with minor changes as you suggested. Thanks!

P.s. The new test passed on my POWER9+kernel-6.10 and Skylake-SP+
kernel-5.14.

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp