[LTP] [PATCH] Revert "pkey01: Adding test for PKEY_DISABLE_EXECUTE"
Martin Doucha
mdoucha@suse.cz
Fri Nov 15 17:06:00 CET 2024
Sorry, I forgot to CC Li in the patch.
On 15. 11. 24 16:44, Martin Doucha wrote:
> This reverts commit d2b8a476c29d52c484b387454082bbc906b0b4f8.
>
> Remove the PKEY_DISABLE_EXECUTE subtest. The function_size() code
> is broken in a way that I cannot easily fix. The function tries
> to calculate the size of a function by finding the first RET
> instruction. However, in 32bit LTP builds, the code gets compiled
> to this:
>
> 0804b690 <function_size>:
> 804b690: 8b 4c 24 04 mov 0x4(%esp),%ecx
> 804b694: 0f b6 01 movzbl (%ecx),%eax
> 804b697: 83 c0 3e add $0x3e,%eax
> 804b69a: 3c 01 cmp $0x1,%al
> 804b69c: 76 1a jbe 804b6b8 <function_size+0x28>
> 804b69e: 89 c8 mov %ecx,%eax
> 804b6a0: 83 c0 01 add $0x1,%eax
> 804b6a3: 0f b6 10 movzbl (%eax),%edx
> 804b6a6: 83 c2 3e add $0x3e,%edx
> 804b6a9: 80 fa 01 cmp $0x1,%dl
> 804b6ac: 77 f2 ja 804b6a0 <function_size+0x10>
> 804b6ae: 29 c8 sub %ecx,%eax
> 804b6b0: 83 c0 10 add $0x10,%eax
> 804b6b3: c3 ret
> 804b6b4: 8d 74 26 00 lea 0x0(%esi,%eiz,1),%esi
> 804b6b8: b8 10 00 00 00 mov $0x10,%eax
> 804b6bd: c3 ret
> 804b6be: 66 90 xchg %ax,%ax
>
> If you look closely enough, you'll notice a C2 byte in add $0x3e,%edx
> instruction on address 804b6a6. The function will assume this byte is
> a RET instruction, return a size that's 22 bytes too short and then
> the code execution inside the executable buffer will run past the end
> of buffer, resulting in a segfault.
>
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
> testcases/kernel/syscalls/pkeys/pkey01.c | 52 ++----------------------
> 1 file changed, 3 insertions(+), 49 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/pkeys/pkey01.c b/testcases/kernel/syscalls/pkeys/pkey01.c
> index c041cbcfd..e49e48846 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-2024 Red Hat, Inc.
> + * Copyright (c) 2019 Red Hat, Inc.
> */
>
> /*\
> @@ -41,7 +41,6 @@
> #define PATH_VM_NRHPS "/proc/sys/vm/nr_hugepages"
>
> static int size;
> -static int execute_supported = 1;
>
> #define PERM_NAME(x) .access_rights = x, .name = #x
> static struct tcase {
> @@ -51,26 +50,14 @@ static struct tcase {
> } tcases[] = {
> {PERM_NAME(PKEY_DISABLE_ACCESS)},
> {PERM_NAME(PKEY_DISABLE_WRITE)},
> - {PERM_NAME(PKEY_DISABLE_EXECUTE)} /* keep it the last */
> };
>
> static void setup(void)
> {
> - int i, fd, pkey;
> + int i, fd;
>
> check_pkey_support();
>
> - pkey = pkey_alloc(0, PKEY_DISABLE_EXECUTE);
> - if (pkey == -1) {
> - if (errno == EINVAL) {
> - tst_res(TINFO, "PKEY_DISABLE_EXECUTE not implemented");
> - 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
> @@ -144,17 +131,6 @@ 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);
> -}
> -
> /*
> * return: 1 if it's safe to quit testing on failure (all following would be
> * TCONF, O otherwise.
> @@ -165,13 +141,6 @@ static int pkey_test(struct tcase *tc, struct mmap_param *mpa)
> char *buffer;
> int pkey, status;
> int fd = mpa->fd;
> - size_t (*func)();
> - size_t func_size = 0;
> -
> - if (!execute_supported && (tc->access_rights == PKEY_DISABLE_EXECUTE)) {
> - tst_res(TCONF, "skip PKEY_DISABLE_EXECUTE test");
> - return 1;
> - }
>
> if (!tst_hugepages && (mpa->flags & MAP_HUGETLB)) {
> tst_res(TCONF, "Skip test on (%s) buffer", flag_to_str(mpa->flags));
> @@ -183,11 +152,6 @@ static int 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");
> @@ -210,10 +174,6 @@ static int 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, "Execute buffer result = %zi", func(func));
> - break;
> }
> exit(0);
> }
> @@ -238,16 +198,10 @@ static int 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 = %zi", func_size);
> - else
> - tst_res(TFAIL, "Execute buffer with unexpected result: %zi", func(func));
> - break;
> }
>
> if (fd >= 0)
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
More information about the ltp
mailing list