[linux-morello] Re: [PATCH V2 5/8] kselftests/arm64/morello: Add mmap() bounds check testcases

On 17-07-2023 06:11, Chaitanya S Prakash wrote:

Reservations are contiguous ranges of virtual addresses that exactly match the bounds of an owning capability which is provided by the kernel. When an owning capability is passed to a syscall, its bounds are first verified against the existing reservations. If the bounds of the capability are found to not match any of the existing reservations, the syscall is expected to fail with a -ERESERVATION error code. Tests to verify this behaviour has been added.

An additional check to verify whether the representable length returned by the capability is greater than or equal to the bounds set for the capability has been added. Finally, signal handler has also been registered to handle invalid memory access.

Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com

tools/testing/selftests/arm64/morello/mmap.c | 116 +++++++++++++++++++ 1 file changed, 116 insertions(+)

diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 4a2a552d47dc..c7f1ddb395c5 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -8,8 +8,13 @@ #include <linux/errno.h> #include <linux/fs.h> #include <linux/fcntl.h> +#include <linux/signal.h> +#include <asm/sigcontext.h> +#include <asm/siginfo.h> +#include <asm/ucontext.h> #include <cheriintrin.h> #include "freestanding.h" +#include "signal_common.h" #define MMAP_SIZE ((1ULL << 16) << 1) /* 64k x 2 */ @@ -19,6 +24,7 @@ #define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02 +static volatile unsigned int signal_status; static inline int probe_mem_range(void *addr, size_t size, int mode) { @@ -37,6 +43,37 @@ static inline int probe_mem_range(void *addr, size_t size, int mode) return 0; } +static void purecap_sigsegv_handler(int n, siginfo_t *si, void *data) +{

• struct ucontext *uc = (struct ucontext *)data;
• ASSERT_EQ(si->si_signo, n);
• TH_LOG("Signal (%d) occurred", n);
• uc->uc_mcontext.pc += 4;
• signal_status = 1;

+}

+static void raise_sigsegv(void) +{

• void *addr;
• long value;
• int retval;
• unsigned int *invalid_addr;

You can leave it "void *addr", so that you don't need to do a cast below.

• int prot = PROT_READ | PROT_WRITE;
• int flags = MAP_PRIVATE | MAP_ANONYMOUS;
• addr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
• ASSERT_GT((unsigned long)addr, 0);
• invalid_addr = (unsigned int *)addr;

You don't need this assignment I think.

• EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE,

• 		     PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
  

• invalid_addr = (unsigned int *)(addr + MMAP_SIZE);
• asm volatile("ldr %w0, [%1]" : "=r" (value) : "C" (invalid_addr) : "memory");
• retval = munmap(addr, MMAP_SIZE);
• ASSERT_EQ(retval, 0);

+}

• /* Simple test to check our ability to create a new anonymous mapping
  • in the virtual address space of the calling process
  */

@@ -324,6 +361,79 @@ static inline void purecap_range_check(void) ASSERT_EQ(retval, 0); } +/* test to verify mmap() behaviour when capability bounds are modified */ +static inline void purecap_mmap_bounds_check(void) +{

• int retval;
• ptraddr_t address;
• size_t res_length;
• void *addr, *addr_cap, *ddc_cap, *ddc_value, *new_addr, *null_cap_addr;

addr is a valid capability throughout this test, so having both addr and addr_cap is a bit confusing.

• int prot = PROT_READ | PROT_WRITE;
• int flags = MAP_PRIVATE | MAP_ANONYMOUS;
• struct sigaction sa;
• /* generate a null-derived capability to be used later */
• addr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
• ASSERT_GT((unsigned long)addr, 0);
• address = cheri_address_get(addr);
• null_cap_addr = (void *)(uintptr_t)address;
• retval = munmap(addr, MMAP_SIZE);
• ASSERT_EQ(retval, 0);
• addr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
• ASSERT_GT((unsigned long)addr, 0);
• /* bounds of capability is increased by using root capability of the DDC

• * register */
  

• ddc_value = cheri_ddc_get();

Don't need ddc_value and ddc_cap. Just use only one variable: ddc_cap.

• ddc_cap = cheri_address_set(ddc_value, cheri_address_get(addr));
• ddc_cap = cheri_bounds_set(ddc_value, 2 * MMAP_SIZE);

Probably you meant cheri_bounds_set(ddc_cap,...) 🙂. Same on the line below.

• ddc_cap = cheri_perms_and(ddc_value, cheri_perms_get(addr));
• EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE,

• 		     PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
  

• retval = munmap(ddc_cap, MMAP_SIZE);
• ASSERT_EQ(retval, 0);
• retval = munmap(addr, MMAP_SIZE);
• ASSERT_EQ(retval, 0);

Is there a particular reason why you munmap the same region twice?

• /* bounds of capability returned by mmap() is reduced, followed by a

• * representable length check */
  

• addr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
• ASSERT_GT((unsigned long)addr, 0);
• addr_cap = cheri_bounds_set(addr, MMAP_SIZE_REDUCED);
• res_length = cheri_representable_length((unsigned long)addr_cap);

cheri_representable_length(size_t len) returns the length that a capability would have after using cheri_bounds_set to set the length to len (assuming appropriate alignment of the base). It's not addr_cap the argument that you want to pass to it AFAICT, maybe like this:

res_length = cheri_representable_length(cheri_length_get(addr_cap));

• EXPECT_GE(res_length, MMAP_SIZE_REDUCED);
• retval = munmap(addr_cap, MMAP_SIZE);
• ASSERT_EQ(retval, -EINVAL);
• retval = munmap(addr, MMAP_SIZE);
• ASSERT_EQ(retval, 0);
• /* overlapping mappings produce a reservation error */
• addr = mmap(null_cap_addr, MMAP_SIZE, prot, flags | MAP_FIXED, -1, 0);
• ASSERT_GT((unsigned long)addr, 0);
• new_addr = mmap(null_cap_addr + MMAP_SIZE_REDUCED, MMAP_SIZE_REDUCED,

• 	prot, flags | MAP_FIXED, -1, 0);
  

• ASSERT_EQ((unsigned long)new_addr, (unsigned long)-ERESERVATION);
• retval = munmap(addr, MMAP_SIZE);
• ASSERT_EQ(retval, 0);
• /* register a signal handler to catch invalid memory access */

I really like your comments for each mechanism that you're testing. I would suggest adding to this comment what the code below actually tests, and then why you need to register a signal handler.

• sigemptyset(&sa.sa_mask);
• sa.sa_handler = (sighandler_t)(void *)purecap_sigsegv_handler;
• sa.sa_flags = SA_SIGINFO;
• sigaction(SIGSEGV, &sa, NULL);
• signal_status = 0;
• raise_sigsegv();
• ASSERT_TRUE(signal_status);
• TH_LOG("Segmenation fault caused by invalid memory access has been handled.");

Typo in Segmentation.

Tudor

+}

• TEST(test_syscall_mmap) { syscall_mmap();

@@ -349,6 +459,11 @@ TEST(test_purecap_range_check) purecap_range_check(); } +TEST(test_purecap_mmap_bounds_check) +{

• purecap_mmap_bounds_check();

+}

• int main(void) { test_syscall_mmap();

@@ -356,5 +471,6 @@ int main(void) test_purecap_map_growsdown(); test_purecap_param_check(); test_purecap_range_check();

• test_purecap_mmap_bounds_check(); return 0; }