Date:   Thu, 18 Aug 2022 17:04:36 +1000
From:   Nicholas Miehlbradt <nicholas@...ux.ibm.com>
To:     Jordan Niethe <jniethe5@...il.com>,
        Russell Currey <ruscur@...sell.cc>,
        linuxppc-dev@...ts.ozlabs.org
Cc:     ajd@...ux.ibm.com, anshuman.khandual@....com,
        aneesh.kumar@...ux.ibm.com, npiggin@...il.com,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH v4 2/2] selftests/powerpc: Add a test for execute-only
 memory

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 <nicholas@...ux.ibm.com>
>>
>> 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 <nicholas@...ux.ibm.com>
>> Signed-off-by: Russell Currey <ruscur@...sell.cc>
>> ---
>> 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");
>> +}
> 

Powered by blists - more mailing lists