[v4,2/2] selftests/powerpc: Add a test for execute-only memory

Commit Message

Russell Currey Aug. 17, 2022, 5:06 a.m. UTC

From: Nicholas Miehlbradt <[e-mail]>

This selftest is designed to cover execute-only protections
on the Radix MMU but will also work with Hash.

The tests are based on those found in pkey_exec_test with modifications
to use the generic mprotect() instead of the pkey variants.

Signed-off-by: Nicholas Miehlbradt <[e-mail]>
Signed-off-by: Russell Currey <[e-mail]>
---
v4: new

 tools/testing/selftests/powerpc/mm/Makefile   |   3 +-
 .../testing/selftests/powerpc/mm/exec_prot.c  | 231 ++++++++++++++++++
 2 files changed, 233 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/mm/exec_prot.c

Comments

Christophe Leroy Aug. 17, 2022, 5:54 a.m. UTC | #1

Le 17/08/2022 à 07:06, Russell Currey a écrit :
> From: Nicholas Miehlbradt <[e-mail]>
> 
> This selftest is designed to cover execute-only protections
> on the Radix MMU but will also work with Hash.
> 
> The tests are based on those found in pkey_exec_test with modifications
> to use the generic mprotect() instead of the pkey variants.
> 
> Signed-off-by: Nicholas Miehlbradt <[e-mail]>
> Signed-off-by: Russell Currey <[e-mail]>
> ---
> v4: new
> 
>   tools/testing/selftests/powerpc/mm/Makefile   |   3 +-
>   .../testing/selftests/powerpc/mm/exec_prot.c  | 231 ++++++++++++++++++

There is a lot of code in common with pkey_exec_prot.c
Isn't there a way to refactor ?

>   2 files changed, 233 insertions(+), 1 deletion(-)
>   create mode 100644 tools/testing/selftests/powerpc/mm/exec_prot.c
> 
> diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
> index 27dc09d0bfee..19dd0b2ea397 100644
> --- a/tools/testing/selftests/powerpc/mm/Makefile
> +++ b/tools/testing/selftests/powerpc/mm/Makefile
> @@ -3,7 +3,7 @@ noarg:
>   	$(MAKE) -C ../
>   
>   TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
> -		  large_vm_fork_separation bad_accesses pkey_exec_prot \
> +		  large_vm_fork_separation bad_accesses exec_prot pkey_exec_prot \
>   		  pkey_siginfo stack_expansion_signal stack_expansion_ldst \
>   		  large_vm_gpr_corruption
>   TEST_PROGS := stress_code_patching.sh
> @@ -22,6 +22,7 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
>   $(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
>   $(OUTPUT)/large_vm_gpr_corruption: CFLAGS += -m64
>   $(OUTPUT)/bad_accesses: CFLAGS += -m64
> +$(OUTPUT)/exec_prot: CFLAGS += -m64
>   $(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
>   $(OUTPUT)/pkey_siginfo: CFLAGS += -m64
>   
> diff --git a/tools/testing/selftests/powerpc/mm/exec_prot.c b/tools/testing/selftests/powerpc/mm/exec_prot.c
> new file mode 100644
> index 000000000000..db75b2225de1
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mm/exec_prot.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2022, Nicholas Miehlbradt, IBM Corporation
> + * based on pkey_exec_prot.c
> + *
> + * Test if applying execute protection on pages works as expected.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <signal.h>
> +
> +#include <unistd.h>
> +#include <sys/mman.h>
> +
> +#include "pkeys.h"
> +
> +
> +#define PPC_INST_NOP	0x60000000
> +#define PPC_INST_TRAP	0x7fe00008
> +#define PPC_INST_BLR	0x4e800020
> +
> +static volatile sig_atomic_t fault_code;
> +static volatile sig_atomic_t remaining_faults;
> +static volatile unsigned int *fault_addr;
> +static unsigned long pgsize, numinsns;
> +static unsigned int *insns;
> +static bool pkeys_supported;
> +
> +static bool is_fault_expected(int fault_code)
> +{
> +	if (fault_code == SEGV_ACCERR)
> +		return true;
> +
> +	/* Assume any pkey error is fine since pkey_exec_prot test covers them */
> +	if (fault_code == SEGV_PKUERR && pkeys_supported)
> +		return true;
> +
> +	return false;
> +}
> +
> +static void trap_handler(int signum, siginfo_t *sinfo, void *ctx)
> +{
> +	/* Check if this fault originated from the expected address */
> +	if (sinfo->si_addr != (void *)fault_addr)
> +		sigsafe_err("got a fault for an unexpected address\n");
> +
> +	_exit(1);
> +}
> +
> +static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
> +{
> +	fault_code = sinfo->si_code;
> +
> +	/* Check if this fault originated from the expected address */
> +	if (sinfo->si_addr != (void *)fault_addr) {
> +		sigsafe_err("got a fault for an unexpected address\n");
> +		_exit(1);
> +	}
> +
> +	/* Check if too many faults have occurred for a single test case */
> +	if (!remaining_faults) {
> +		sigsafe_err("got too many faults for the same address\n");
> +		_exit(1);
> +	}
> +
> +
> +	/* Restore permissions in order to continue */
> +	if (is_fault_expected(fault_code)) {
> +		if (mprotect(insns, pgsize, PROT_READ | PROT_WRITE | PROT_EXEC)) {
> +			sigsafe_err("failed to set access permissions\n");
> +			_exit(1);
> +		}
> +	} else {
> +		sigsafe_err("got a fault with an unexpected code\n");
> +		_exit(1);
> +	}
> +
> +	remaining_faults--;
> +}
> +
> +static int check_exec_fault(int rights)
> +{
> +	/*
> +	 * Jump to the executable region.
> +	 *
> +	 * The first iteration also checks if the overwrite of the
> +	 * first instruction word from a trap to a no-op succeeded.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 0;
> +	if (!(rights & PROT_EXEC))
> +		remaining_faults = 1;
> +
> +	FAIL_IF(mprotect(insns, pgsize, rights) != 0);
> +	asm volatile("mtctr	%0; bctrl" : : "r"(insns));
> +
> +	FAIL_IF(remaining_faults != 0);
> +	if (!(rights & PROT_EXEC))
> +		FAIL_IF(!is_fault_expected(fault_code));
> +
> +	return 0;
> +}
> +
> +static int test(void)
> +{
> +	struct sigaction segv_act, trap_act;
> +	int i;
> +
> +	/* Skip the test if the CPU doesn't support Radix */
> +	SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_00));
> +
> +	/* Check if pkeys are supported */
> +	pkeys_supported = pkeys_unsupported() == 0;
> +
> +	/* Setup SIGSEGV handler */
> +	segv_act.sa_handler = 0;
> +	segv_act.sa_sigaction = segv_handler;
> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &segv_act.sa_mask) != 0);
> +	segv_act.sa_flags = SA_SIGINFO;
> +	segv_act.sa_restorer = 0;
> +	FAIL_IF(sigaction(SIGSEGV, &segv_act, NULL) != 0);
> +
> +	/* Setup SIGTRAP handler */
> +	trap_act.sa_handler = 0;
> +	trap_act.sa_sigaction = trap_handler;
> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &trap_act.sa_mask) != 0);
> +	trap_act.sa_flags = SA_SIGINFO;
> +	trap_act.sa_restorer = 0;
> +	FAIL_IF(sigaction(SIGTRAP, &trap_act, NULL) != 0);
> +
> +	/* Setup executable region */
> +	pgsize = getpagesize();
> +	numinsns = pgsize / sizeof(unsigned int);
> +	insns = (unsigned int *)mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
> +				      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	FAIL_IF(insns == MAP_FAILED);
> +
> +	/* Write the instruction words */
> +	for (i = 1; i < numinsns - 1; i++)
> +		insns[i] = PPC_INST_NOP;
> +
> +	/*
> +	 * Set the first instruction as an unconditional trap. If
> +	 * the last write to this address succeeds, this should
> +	 * get overwritten by a no-op.
> +	 */
> +	insns[0] = PPC_INST_TRAP;
> +
> +	/*
> +	 * Later, to jump to the executable region, we use a branch
> +	 * and link instruction (bctrl) which sets the return address
> +	 * automatically in LR. Use that to return back.
> +	 */
> +	insns[numinsns - 1] = PPC_INST_BLR;
> +
> +	/*
> +	 * Pick the first instruction's address from the executable
> +	 * region.
> +	 */
> +	fault_addr = insns;
> +
> +	/*
> +	 * Read an instruction word from the address when the page
> +	 * is execute only. This should generate an access fault.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 1;
> +	printf("Testing read on --x, should fault...");
> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
> +	i = *fault_addr;
> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
> +	printf("ok!\n");
> +
> +	/*
> +	 * Write an instruction word to the address when the page
> +	 * execute only. This should also generate an access fault.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 1;
> +	printf("Testing write on --x, should fault...");
> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
> +	*fault_addr = PPC_INST_NOP;
> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on ---, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_NONE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on r--, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_READ));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on -w-, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_WRITE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on rw-, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on --x, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on r-x, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on -wx, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_WRITE | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on rwx, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	/* Cleanup */
> +	FAIL_IF(munmap((void *)insns, pgsize));
> +
> +	return 0;
> +}
> +
> +int main(void)
> +{
> +	return test_harness(test, "exec_prot");
> +}

Jordan Niethe Aug. 17, 2022, 6:15 a.m. UTC | #2

On Wed, 2022-08-17 at 15:06 +1000, Russell Currey wrote:
> From: Nicholas Miehlbradt <[e-mail]>
> 
> This selftest is designed to cover execute-only protections
> on the Radix MMU but will also work with Hash.
> 
> The tests are based on those found in pkey_exec_test with modifications
> to use the generic mprotect() instead of the pkey variants.

Would it make sense to rename pkey_exec_test to exec_test and have this test be apart of that?

> 
> Signed-off-by: Nicholas Miehlbradt <[e-mail]>
> Signed-off-by: Russell Currey <[e-mail]>
> ---
> v4: new
> 
>  tools/testing/selftests/powerpc/mm/Makefile   |   3 +-
>  .../testing/selftests/powerpc/mm/exec_prot.c  | 231 ++++++++++++++++++
>  2 files changed, 233 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/mm/exec_prot.c
> 
> diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
> index 27dc09d0bfee..19dd0b2ea397 100644
> --- a/tools/testing/selftests/powerpc/mm/Makefile
> +++ b/tools/testing/selftests/powerpc/mm/Makefile
> @@ -3,7 +3,7 @@ noarg:
>  	$(MAKE) -C ../
>  
>  TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
> -		  large_vm_fork_separation bad_accesses pkey_exec_prot \
> +		  large_vm_fork_separation bad_accesses exec_prot pkey_exec_prot \
>  		  pkey_siginfo stack_expansion_signal stack_expansion_ldst \
>  		  large_vm_gpr_corruption
>  TEST_PROGS := stress_code_patching.sh
> @@ -22,6 +22,7 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
>  $(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
>  $(OUTPUT)/large_vm_gpr_corruption: CFLAGS += -m64
>  $(OUTPUT)/bad_accesses: CFLAGS += -m64
> +$(OUTPUT)/exec_prot: CFLAGS += -m64
>  $(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
>  $(OUTPUT)/pkey_siginfo: CFLAGS += -m64
>  
> diff --git a/tools/testing/selftests/powerpc/mm/exec_prot.c b/tools/testing/selftests/powerpc/mm/exec_prot.c
> new file mode 100644
> index 000000000000..db75b2225de1
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mm/exec_prot.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2022, Nicholas Miehlbradt, IBM Corporation
> + * based on pkey_exec_prot.c
> + *
> + * Test if applying execute protection on pages works as expected.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <signal.h>
> +
> +#include <unistd.h>
> +#include <sys/mman.h>
> +
> +#include "pkeys.h"
> +
> +
> +#define PPC_INST_NOP	0x60000000
> +#define PPC_INST_TRAP	0x7fe00008
> +#define PPC_INST_BLR	0x4e800020
> +
> +static volatile sig_atomic_t fault_code;
> +static volatile sig_atomic_t remaining_faults;
> +static volatile unsigned int *fault_addr;
> +static unsigned long pgsize, numinsns;
> +static unsigned int *insns;
> +static bool pkeys_supported;
> +
> +static bool is_fault_expected(int fault_code)
> +{
> +	if (fault_code == SEGV_ACCERR)
> +		return true;
> +
> +	/* Assume any pkey error is fine since pkey_exec_prot test covers them */
> +	if (fault_code == SEGV_PKUERR && pkeys_supported)
> +		return true;
> +
> +	return false;
> +}
> +
> +static void trap_handler(int signum, siginfo_t *sinfo, void *ctx)
> +{
> +	/* Check if this fault originated from the expected address */
> +	if (sinfo->si_addr != (void *)fault_addr)
> +		sigsafe_err("got a fault for an unexpected address\n");
> +
> +	_exit(1);
> +}
> +
> +static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
> +{
> +	fault_code = sinfo->si_code;
> +
> +	/* Check if this fault originated from the expected address */
> +	if (sinfo->si_addr != (void *)fault_addr) {
> +		sigsafe_err("got a fault for an unexpected address\n");
> +		_exit(1);
> +	}
> +
> +	/* Check if too many faults have occurred for a single test case */
> +	if (!remaining_faults) {
> +		sigsafe_err("got too many faults for the same address\n");
> +		_exit(1);
> +	}
> +
> +
> +	/* Restore permissions in order to continue */
> +	if (is_fault_expected(fault_code)) {
> +		if (mprotect(insns, pgsize, PROT_READ | PROT_WRITE | PROT_EXEC)) {
> +			sigsafe_err("failed to set access permissions\n");
> +			_exit(1);
> +		}
> +	} else {
> +		sigsafe_err("got a fault with an unexpected code\n");
> +		_exit(1);
> +	}
> +
> +	remaining_faults--;
> +}
> +
> +static int check_exec_fault(int rights)
> +{
> +	/*
> +	 * Jump to the executable region.
> +	 *
> +	 * The first iteration also checks if the overwrite of the
> +	 * first instruction word from a trap to a no-op succeeded.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 0;
> +	if (!(rights & PROT_EXEC))
> +		remaining_faults = 1;
> +
> +	FAIL_IF(mprotect(insns, pgsize, rights) != 0);
> +	asm volatile("mtctr	%0; bctrl" : : "r"(insns));
> +
> +	FAIL_IF(remaining_faults != 0);
> +	if (!(rights & PROT_EXEC))
> +		FAIL_IF(!is_fault_expected(fault_code));
> +
> +	return 0;
> +}
> +
> +static int test(void)
> +{
> +	struct sigaction segv_act, trap_act;
> +	int i;
> +
> +	/* Skip the test if the CPU doesn't support Radix */
> +	SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_00));
> +
> +	/* Check if pkeys are supported */
> +	pkeys_supported = pkeys_unsupported() == 0;
> +
> +	/* Setup SIGSEGV handler */
> +	segv_act.sa_handler = 0;
> +	segv_act.sa_sigaction = segv_handler;
> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &segv_act.sa_mask) != 0);
> +	segv_act.sa_flags = SA_SIGINFO;
> +	segv_act.sa_restorer = 0;
> +	FAIL_IF(sigaction(SIGSEGV, &segv_act, NULL) != 0);
> +
> +	/* Setup SIGTRAP handler */
> +	trap_act.sa_handler = 0;
> +	trap_act.sa_sigaction = trap_handler;
> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &trap_act.sa_mask) != 0);
> +	trap_act.sa_flags = SA_SIGINFO;
> +	trap_act.sa_restorer = 0;
> +	FAIL_IF(sigaction(SIGTRAP, &trap_act, NULL) != 0);
> +
> +	/* Setup executable region */
> +	pgsize = getpagesize();
> +	numinsns = pgsize / sizeof(unsigned int);
> +	insns = (unsigned int *)mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
> +				      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	FAIL_IF(insns == MAP_FAILED);
> +
> +	/* Write the instruction words */
> +	for (i = 1; i < numinsns - 1; i++)
> +		insns[i] = PPC_INST_NOP;
> +
> +	/*
> +	 * Set the first instruction as an unconditional trap. If
> +	 * the last write to this address succeeds, this should
> +	 * get overwritten by a no-op.
> +	 */
> +	insns[0] = PPC_INST_TRAP;
> +
> +	/*
> +	 * Later, to jump to the executable region, we use a branch
> +	 * and link instruction (bctrl) which sets the return address
> +	 * automatically in LR. Use that to return back.
> +	 */
> +	insns[numinsns - 1] = PPC_INST_BLR;
> +
> +	/*
> +	 * Pick the first instruction's address from the executable
> +	 * region.
> +	 */
> +	fault_addr = insns;
> +
> +	/*
> +	 * Read an instruction word from the address when the page
> +	 * is execute only. This should generate an access fault.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 1;
> +	printf("Testing read on --x, should fault...");
> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
> +	i = *fault_addr;
> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
> +	printf("ok!\n");
> +
> +	/*
> +	 * Write an instruction word to the address when the page
> +	 * execute only. This should also generate an access fault.
> +	 */
> +	fault_code = -1;
> +	remaining_faults = 1;
> +	printf("Testing write on --x, should fault...");
> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
> +	*fault_addr = PPC_INST_NOP;
> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on ---, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_NONE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on r--, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_READ));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on -w-, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_WRITE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on rw-, should fault...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on --x, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on r-x, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on -wx, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_WRITE | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	printf("Testing exec on rwx, should succeed...");
> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE | PROT_EXEC));
> +	printf("ok!\n");
> +
> +	/* Cleanup */
> +	FAIL_IF(munmap((void *)insns, pgsize));
> +
> +	return 0;
> +}
> +
> +int main(void)
> +{
> +	return test_harness(test, "exec_prot");
> +}

Nicholas Miehlbradt Aug. 18, 2022, 7:04 a.m. UTC | #3

On 17/8/2022 4:15 pm, Jordan Niethe wrote:
> On Wed, 2022-08-17 at 15:06 +1000, Russell Currey wrote:
>> From: Nicholas Miehlbradt <[e-mail]>
>>
>> This selftest is designed to cover execute-only protections
>> on the Radix MMU but will also work with Hash.
>>
>> The tests are based on those found in pkey_exec_test with modifications
>> to use the generic mprotect() instead of the pkey variants.
> 
> Would it make sense to rename pkey_exec_test to exec_test and have this test be apart of that?
> 
I think might make it unnecessarily complex. The checks needed when 
testing with pkeys would mean that it would be necessary to check if 
pkeys are enabled and choose which set of tests to run depending on the 
result. The differences are substantial enough that it would be 
challenging to combine them into a single set of tests.

>>
>> Signed-off-by: Nicholas Miehlbradt <[e-mail]>
>> Signed-off-by: Russell Currey <[e-mail]>
>> ---
>> v4: new
>>
>>   tools/testing/selftests/powerpc/mm/Makefile   |   3 +-
>>   .../testing/selftests/powerpc/mm/exec_prot.c  | 231 ++++++++++++++++++
>>   2 files changed, 233 insertions(+), 1 deletion(-)
>>   create mode 100644 tools/testing/selftests/powerpc/mm/exec_prot.c
>>
>> diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
>> index 27dc09d0bfee..19dd0b2ea397 100644
>> --- a/tools/testing/selftests/powerpc/mm/Makefile
>> +++ b/tools/testing/selftests/powerpc/mm/Makefile
>> @@ -3,7 +3,7 @@ noarg:
>>   	$(MAKE) -C ../
>>   
>>   TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
>> -		  large_vm_fork_separation bad_accesses pkey_exec_prot \
>> +		  large_vm_fork_separation bad_accesses exec_prot pkey_exec_prot \
>>   		  pkey_siginfo stack_expansion_signal stack_expansion_ldst \
>>   		  large_vm_gpr_corruption
>>   TEST_PROGS := stress_code_patching.sh
>> @@ -22,6 +22,7 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
>>   $(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
>>   $(OUTPUT)/large_vm_gpr_corruption: CFLAGS += -m64
>>   $(OUTPUT)/bad_accesses: CFLAGS += -m64
>> +$(OUTPUT)/exec_prot: CFLAGS += -m64
>>   $(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
>>   $(OUTPUT)/pkey_siginfo: CFLAGS += -m64
>>   
>> diff --git a/tools/testing/selftests/powerpc/mm/exec_prot.c b/tools/testing/selftests/powerpc/mm/exec_prot.c
>> new file mode 100644
>> index 000000000000..db75b2225de1
>> --- /dev/null
>> +++ b/tools/testing/selftests/powerpc/mm/exec_prot.c
>> @@ -0,0 +1,231 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * Copyright 2022, Nicholas Miehlbradt, IBM Corporation
>> + * based on pkey_exec_prot.c
>> + *
>> + * Test if applying execute protection on pages works as expected.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <signal.h>
>> +
>> +#include <unistd.h>
>> +#include <sys/mman.h>
>> +
>> +#include "pkeys.h"
>> +
>> +
>> +#define PPC_INST_NOP	0x60000000
>> +#define PPC_INST_TRAP	0x7fe00008
>> +#define PPC_INST_BLR	0x4e800020
>> +
>> +static volatile sig_atomic_t fault_code;
>> +static volatile sig_atomic_t remaining_faults;
>> +static volatile unsigned int *fault_addr;
>> +static unsigned long pgsize, numinsns;
>> +static unsigned int *insns;
>> +static bool pkeys_supported;
>> +
>> +static bool is_fault_expected(int fault_code)
>> +{
>> +	if (fault_code == SEGV_ACCERR)
>> +		return true;
>> +
>> +	/* Assume any pkey error is fine since pkey_exec_prot test covers them */
>> +	if (fault_code == SEGV_PKUERR && pkeys_supported)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +static void trap_handler(int signum, siginfo_t *sinfo, void *ctx)
>> +{
>> +	/* Check if this fault originated from the expected address */
>> +	if (sinfo->si_addr != (void *)fault_addr)
>> +		sigsafe_err("got a fault for an unexpected address\n");
>> +
>> +	_exit(1);
>> +}
>> +
>> +static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
>> +{
>> +	fault_code = sinfo->si_code;
>> +
>> +	/* Check if this fault originated from the expected address */
>> +	if (sinfo->si_addr != (void *)fault_addr) {
>> +		sigsafe_err("got a fault for an unexpected address\n");
>> +		_exit(1);
>> +	}
>> +
>> +	/* Check if too many faults have occurred for a single test case */
>> +	if (!remaining_faults) {
>> +		sigsafe_err("got too many faults for the same address\n");
>> +		_exit(1);
>> +	}
>> +
>> +
>> +	/* Restore permissions in order to continue */
>> +	if (is_fault_expected(fault_code)) {
>> +		if (mprotect(insns, pgsize, PROT_READ | PROT_WRITE | PROT_EXEC)) {
>> +			sigsafe_err("failed to set access permissions\n");
>> +			_exit(1);
>> +		}
>> +	} else {
>> +		sigsafe_err("got a fault with an unexpected code\n");
>> +		_exit(1);
>> +	}
>> +
>> +	remaining_faults--;
>> +}
>> +
>> +static int check_exec_fault(int rights)
>> +{
>> +	/*
>> +	 * Jump to the executable region.
>> +	 *
>> +	 * The first iteration also checks if the overwrite of the
>> +	 * first instruction word from a trap to a no-op succeeded.
>> +	 */
>> +	fault_code = -1;
>> +	remaining_faults = 0;
>> +	if (!(rights & PROT_EXEC))
>> +		remaining_faults = 1;
>> +
>> +	FAIL_IF(mprotect(insns, pgsize, rights) != 0);
>> +	asm volatile("mtctr	%0; bctrl" : : "r"(insns));
>> +
>> +	FAIL_IF(remaining_faults != 0);
>> +	if (!(rights & PROT_EXEC))
>> +		FAIL_IF(!is_fault_expected(fault_code));
>> +
>> +	return 0;
>> +}
>> +
>> +static int test(void)
>> +{
>> +	struct sigaction segv_act, trap_act;
>> +	int i;
>> +
>> +	/* Skip the test if the CPU doesn't support Radix */
>> +	SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_00));
>> +
>> +	/* Check if pkeys are supported */
>> +	pkeys_supported = pkeys_unsupported() == 0;
>> +
>> +	/* Setup SIGSEGV handler */
>> +	segv_act.sa_handler = 0;
>> +	segv_act.sa_sigaction = segv_handler;
>> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &segv_act.sa_mask) != 0);
>> +	segv_act.sa_flags = SA_SIGINFO;
>> +	segv_act.sa_restorer = 0;
>> +	FAIL_IF(sigaction(SIGSEGV, &segv_act, NULL) != 0);
>> +
>> +	/* Setup SIGTRAP handler */
>> +	trap_act.sa_handler = 0;
>> +	trap_act.sa_sigaction = trap_handler;
>> +	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &trap_act.sa_mask) != 0);
>> +	trap_act.sa_flags = SA_SIGINFO;
>> +	trap_act.sa_restorer = 0;
>> +	FAIL_IF(sigaction(SIGTRAP, &trap_act, NULL) != 0);
>> +
>> +	/* Setup executable region */
>> +	pgsize = getpagesize();
>> +	numinsns = pgsize / sizeof(unsigned int);
>> +	insns = (unsigned int *)mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
>> +				      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> +	FAIL_IF(insns == MAP_FAILED);
>> +
>> +	/* Write the instruction words */
>> +	for (i = 1; i < numinsns - 1; i++)
>> +		insns[i] = PPC_INST_NOP;
>> +
>> +	/*
>> +	 * Set the first instruction as an unconditional trap. If
>> +	 * the last write to this address succeeds, this should
>> +	 * get overwritten by a no-op.
>> +	 */
>> +	insns[0] = PPC_INST_TRAP;
>> +
>> +	/*
>> +	 * Later, to jump to the executable region, we use a branch
>> +	 * and link instruction (bctrl) which sets the return address
>> +	 * automatically in LR. Use that to return back.
>> +	 */
>> +	insns[numinsns - 1] = PPC_INST_BLR;
>> +
>> +	/*
>> +	 * Pick the first instruction's address from the executable
>> +	 * region.
>> +	 */
>> +	fault_addr = insns;
>> +
>> +	/*
>> +	 * Read an instruction word from the address when the page
>> +	 * is execute only. This should generate an access fault.
>> +	 */
>> +	fault_code = -1;
>> +	remaining_faults = 1;
>> +	printf("Testing read on --x, should fault...");
>> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
>> +	i = *fault_addr;
>> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
>> +	printf("ok!\n");
>> +
>> +	/*
>> +	 * Write an instruction word to the address when the page
>> +	 * execute only. This should also generate an access fault.
>> +	 */
>> +	fault_code = -1;
>> +	remaining_faults = 1;
>> +	printf("Testing write on --x, should fault...");
>> +	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
>> +	*fault_addr = PPC_INST_NOP;
>> +	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on ---, should fault...");
>> +	FAIL_IF(check_exec_fault(PROT_NONE));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on r--, should fault...");
>> +	FAIL_IF(check_exec_fault(PROT_READ));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on -w-, should fault...");
>> +	FAIL_IF(check_exec_fault(PROT_WRITE));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on rw-, should fault...");
>> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on --x, should succeed...");
>> +	FAIL_IF(check_exec_fault(PROT_EXEC));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on r-x, should succeed...");
>> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_EXEC));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on -wx, should succeed...");
>> +	FAIL_IF(check_exec_fault(PROT_WRITE | PROT_EXEC));
>> +	printf("ok!\n");
>> +
>> +	printf("Testing exec on rwx, should succeed...");
>> +	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE | PROT_EXEC));
>> +	printf("ok!\n");
>> +
>> +	/* Cleanup */
>> +	FAIL_IF(munmap((void *)insns, pgsize));
>> +
>> +	return 0;
>> +}
>> +
>> +int main(void)
>> +{
>> +	return test_harness(test, "exec_prot");
>> +}
>

Michael Ellerman Aug. 20, 2022, 6:30 a.m. UTC | #4

Nicholas Miehlbradt <[e-mail]> writes:
> On 17/8/2022 4:15 pm, Jordan Niethe wrote:
>> On Wed, 2022-08-17 at 15:06 +1000, Russell Currey wrote:
>>> From: Nicholas Miehlbradt <[e-mail]>
>>>
>>> This selftest is designed to cover execute-only protections
>>> on the Radix MMU but will also work with Hash.
>>>
>>> The tests are based on those found in pkey_exec_test with modifications
>>> to use the generic mprotect() instead of the pkey variants.
>> 
>> Would it make sense to rename pkey_exec_test to exec_test and have this test be apart of that?
>> 
> I think might make it unnecessarily complex. The checks needed when 
> testing with pkeys would mean that it would be necessary to check if 
> pkeys are enabled and choose which set of tests to run depending on the 
> result. The differences are substantial enough that it would be 
> challenging to combine them into a single set of tests.

Yeah I think I agree. Having each test stand on its own is nice for
debugging also.

In general I'm less bothered about code duplication in tests. We should
try and share code where we can, but it's more important that we have
tests at all, rather than blocking new tests because they duplicate some
code from another test.

So I'm inclined to merge this as-is, we can always refactor it to share
code in future if we think there's enough commonality to warrant it.

cheers

Patch

diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
index 27dc09d0bfee..19dd0b2ea397 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -3,7 +3,7 @@  noarg:
 	$(MAKE) -C ../
 
 TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
-		  large_vm_fork_separation bad_accesses pkey_exec_prot \
+		  large_vm_fork_separation bad_accesses exec_prot pkey_exec_prot \
 		  pkey_siginfo stack_expansion_signal stack_expansion_ldst \
 		  large_vm_gpr_corruption
 TEST_PROGS := stress_code_patching.sh
@@ -22,6 +22,7 @@  $(OUTPUT)/wild_bctr: CFLAGS += -m64
 $(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
 $(OUTPUT)/large_vm_gpr_corruption: CFLAGS += -m64
 $(OUTPUT)/bad_accesses: CFLAGS += -m64
+$(OUTPUT)/exec_prot: CFLAGS += -m64
 $(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
 $(OUTPUT)/pkey_siginfo: CFLAGS += -m64
 
diff --git a/tools/testing/selftests/powerpc/mm/exec_prot.c b/tools/testing/selftests/powerpc/mm/exec_prot.c
new file mode 100644
index 000000000000..db75b2225de1
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/exec_prot.c
@@ -0,0 +1,231 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2022, Nicholas Miehlbradt, IBM Corporation
+ * based on pkey_exec_prot.c
+ *
+ * Test if applying execute protection on pages works as expected.
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <signal.h>
+
+#include <unistd.h>
+#include <sys/mman.h>
+
+#include "pkeys.h"
+
+
+#define PPC_INST_NOP	0x60000000
+#define PPC_INST_TRAP	0x7fe00008
+#define PPC_INST_BLR	0x4e800020
+
+static volatile sig_atomic_t fault_code;
+static volatile sig_atomic_t remaining_faults;
+static volatile unsigned int *fault_addr;
+static unsigned long pgsize, numinsns;
+static unsigned int *insns;
+static bool pkeys_supported;
+
+static bool is_fault_expected(int fault_code)
+{
+	if (fault_code == SEGV_ACCERR)
+		return true;
+
+	/* Assume any pkey error is fine since pkey_exec_prot test covers them */
+	if (fault_code == SEGV_PKUERR && pkeys_supported)
+		return true;
+
+	return false;
+}
+
+static void trap_handler(int signum, siginfo_t *sinfo, void *ctx)
+{
+	/* Check if this fault originated from the expected address */
+	if (sinfo->si_addr != (void *)fault_addr)
+		sigsafe_err("got a fault for an unexpected address\n");
+
+	_exit(1);
+}
+
+static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
+{
+	fault_code = sinfo->si_code;
+
+	/* Check if this fault originated from the expected address */
+	if (sinfo->si_addr != (void *)fault_addr) {
+		sigsafe_err("got a fault for an unexpected address\n");
+		_exit(1);
+	}
+
+	/* Check if too many faults have occurred for a single test case */
+	if (!remaining_faults) {
+		sigsafe_err("got too many faults for the same address\n");
+		_exit(1);
+	}
+
+
+	/* Restore permissions in order to continue */
+	if (is_fault_expected(fault_code)) {
+		if (mprotect(insns, pgsize, PROT_READ | PROT_WRITE | PROT_EXEC)) {
+			sigsafe_err("failed to set access permissions\n");
+			_exit(1);
+		}
+	} else {
+		sigsafe_err("got a fault with an unexpected code\n");
+		_exit(1);
+	}
+
+	remaining_faults--;
+}
+
+static int check_exec_fault(int rights)
+{
+	/*
+	 * Jump to the executable region.
+	 *
+	 * The first iteration also checks if the overwrite of the
+	 * first instruction word from a trap to a no-op succeeded.
+	 */
+	fault_code = -1;
+	remaining_faults = 0;
+	if (!(rights & PROT_EXEC))
+		remaining_faults = 1;
+
+	FAIL_IF(mprotect(insns, pgsize, rights) != 0);
+	asm volatile("mtctr	%0; bctrl" : : "r"(insns));
+
+	FAIL_IF(remaining_faults != 0);
+	if (!(rights & PROT_EXEC))
+		FAIL_IF(!is_fault_expected(fault_code));
+
+	return 0;
+}
+
+static int test(void)
+{
+	struct sigaction segv_act, trap_act;
+	int i;
+
+	/* Skip the test if the CPU doesn't support Radix */
+	SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_00));
+
+	/* Check if pkeys are supported */
+	pkeys_supported = pkeys_unsupported() == 0;
+
+	/* Setup SIGSEGV handler */
+	segv_act.sa_handler = 0;
+	segv_act.sa_sigaction = segv_handler;
+	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &segv_act.sa_mask) != 0);
+	segv_act.sa_flags = SA_SIGINFO;
+	segv_act.sa_restorer = 0;
+	FAIL_IF(sigaction(SIGSEGV, &segv_act, NULL) != 0);
+
+	/* Setup SIGTRAP handler */
+	trap_act.sa_handler = 0;
+	trap_act.sa_sigaction = trap_handler;
+	FAIL_IF(sigprocmask(SIG_SETMASK, 0, &trap_act.sa_mask) != 0);
+	trap_act.sa_flags = SA_SIGINFO;
+	trap_act.sa_restorer = 0;
+	FAIL_IF(sigaction(SIGTRAP, &trap_act, NULL) != 0);
+
+	/* Setup executable region */
+	pgsize = getpagesize();
+	numinsns = pgsize / sizeof(unsigned int);
+	insns = (unsigned int *)mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
+				      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	FAIL_IF(insns == MAP_FAILED);
+
+	/* Write the instruction words */
+	for (i = 1; i < numinsns - 1; i++)
+		insns[i] = PPC_INST_NOP;
+
+	/*
+	 * Set the first instruction as an unconditional trap. If
+	 * the last write to this address succeeds, this should
+	 * get overwritten by a no-op.
+	 */
+	insns[0] = PPC_INST_TRAP;
+
+	/*
+	 * Later, to jump to the executable region, we use a branch
+	 * and link instruction (bctrl) which sets the return address
+	 * automatically in LR. Use that to return back.
+	 */
+	insns[numinsns - 1] = PPC_INST_BLR;
+
+	/*
+	 * Pick the first instruction's address from the executable
+	 * region.
+	 */
+	fault_addr = insns;
+
+	/*
+	 * Read an instruction word from the address when the page
+	 * is execute only. This should generate an access fault.
+	 */
+	fault_code = -1;
+	remaining_faults = 1;
+	printf("Testing read on --x, should fault...");
+	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
+	i = *fault_addr;
+	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
+	printf("ok!\n");
+
+	/*
+	 * Write an instruction word to the address when the page
+	 * execute only. This should also generate an access fault.
+	 */
+	fault_code = -1;
+	remaining_faults = 1;
+	printf("Testing write on --x, should fault...");
+	FAIL_IF(mprotect(insns, pgsize, PROT_EXEC) != 0);
+	*fault_addr = PPC_INST_NOP;
+	FAIL_IF(remaining_faults != 0 || !is_fault_expected(fault_code));
+	printf("ok!\n");
+
+	printf("Testing exec on ---, should fault...");
+	FAIL_IF(check_exec_fault(PROT_NONE));
+	printf("ok!\n");
+
+	printf("Testing exec on r--, should fault...");
+	FAIL_IF(check_exec_fault(PROT_READ));
+	printf("ok!\n");
+
+	printf("Testing exec on -w-, should fault...");
+	FAIL_IF(check_exec_fault(PROT_WRITE));
+	printf("ok!\n");
+
+	printf("Testing exec on rw-, should fault...");
+	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE));
+	printf("ok!\n");
+
+	printf("Testing exec on --x, should succeed...");
+	FAIL_IF(check_exec_fault(PROT_EXEC));
+	printf("ok!\n");
+
+	printf("Testing exec on r-x, should succeed...");
+	FAIL_IF(check_exec_fault(PROT_READ | PROT_EXEC));
+	printf("ok!\n");
+
+	printf("Testing exec on -wx, should succeed...");
+	FAIL_IF(check_exec_fault(PROT_WRITE | PROT_EXEC));
+	printf("ok!\n");
+
+	printf("Testing exec on rwx, should succeed...");
+	FAIL_IF(check_exec_fault(PROT_READ | PROT_WRITE | PROT_EXEC));
+	printf("ok!\n");
+
+	/* Cleanup */
+	FAIL_IF(munmap((void *)insns, pgsize));
+
+	return 0;
+}
+
+int main(void)
+{
+	return test_harness(test, "exec_prot");
+}