Syscalls operating on memory mappings manage their address space via owning capabilities. They must adhere to a certain set of rules[1] in order to ensure memory safety. Address space management syscalls are only allowed to manipulate mappings that are within the range of the owning capability and have appropriate permissions.
Tests to validate the capability's tag, bounds, range as well as permissions have been added. Finally, as certain flags and syscalls conflict with the reservation model or lack implementation, a check to verify appropriate handling of the same has also been added.
Review branch: https://git.morello-project.org/chaitanya_prakash/linux/-/tree/review/pureca...
This patch series has been tested on: https://git.morello-project.org/amitdaniel/linux/-/tree/review/purecap_mm_re...
[1]https://git.morello-project.org/morello/kernel/linux/-/wikis/Morello-pure-ca...
Changes in V5: - Added representability testcase. - Removed global struct reg_data and called get_pagesize() with auxv passed in main(). - Removed VERIFY_ERRNO macro and made use of extended EXPECT_EQ - As helper functions have been removed, the inline attribute line is of no use and has been deleted. - Used a common mapping to avoid creating and destroying mappings repeatedly. - Removed positive testcases as they are not unique to PCuABI - Corrected the error code to reflect -ENOMEM instead of -ERESERVATION when mremap() tries to move a mapping without MREMAP_MAYMOVE flag - Reworded commit messages and restructured code.
Changes in V4: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
-Corrected subject of cover letter
Changes in V3: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
- Added get_pagesize() function and VERRIFY_ERRNO() macro - Added LoadCap and StoreCap permissions testcase - Added validity_tag_check testcases - Added reservation tests - Renamed variable "addr" to "ptr" to avoid confusion when manipulating both addresses and capabilities - Cleaned up syscall_mmap and syscall_mmap2 testcases - Restructured code into testcases that check tags, range, bounds and permissions - Improved range_check testcases - Improved commit messages - Removed helper functions, tests directly written in testcase functions - Removed signal handling and ddc register testcases
Changes in V2: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
- Added link to the review branch - Removed unnecessary whitespace
Changes in V1: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
Amit Daniel Kachhap (1): kselftests/arm64: morello: mmap: Test unrepresentable addresses
Chaitanya S Prakash (10): kselftests/arm64: morello: Create wrapper functions for frequently invoked syscalls kselftests/arm64: morello: Add get_pagesize() function kselftests/arm64: morello: mmap: Clean up existing testcases kselftests/arm64: morello: mmap: Add MAP_GROWSDOWN testcase kselftests/arm64: morello: mmap: Add validity tag check testcases kselftests/arm64: morello: mmap: Add capability range testcases kselftests/arm64: morello: mmap: Add mmap() bounds check testcases kselftests/arm64: morello: mmap: Add mremap() bounds check testcases kselftests/arm64: morello: mmap: Add permission check testcases kselftests/arm64: morello: mmap: Add brk() testcase
.../selftests/arm64/morello/bootstrap.c | 13 - .../selftests/arm64/morello/freestanding.c | 15 + .../selftests/arm64/morello/freestanding.h | 74 ++- tools/testing/selftests/arm64/morello/mmap.c | 533 +++++++++++++++++- 4 files changed, 590 insertions(+), 45 deletions(-)
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Wrapper functions for syscalls that are frequently invoked are added in order to improve usability. Making use of the newly added wrapper functions, Morello kselftests can be re-written in a manner that avoids directly interacting with syscalls. Required datatypes have been defined.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- .../selftests/arm64/morello/freestanding.h | 59 ++++++++++++++++++- tools/testing/selftests/arm64/morello/mmap.c | 11 ++-- 2 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..901521bde55d 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -21,6 +21,7 @@ typedef __kernel_timer_t timer_t; typedef __kernel_clockid_t clockid_t; typedef __kernel_uid_t uid_t; typedef __kernel_mode_t mode_t; +typedef __kernel_off_t off_t; #ifndef __clang__ typedef __uintcap_t uintcap_t; #endif @@ -160,6 +161,16 @@ static inline ssize_t write(int fd, const void *buf, size_t count) return syscall(__NR_write, fd, buf, count); }
+static inline off_t lseek(int fd, off_t offset, int whence) +{ + return syscall(__NR_lseek, fd, offset, whence); +} + +static inline int openat(int dirfd, const char *pathname, int flags, mode_t mode) +{ + return syscall(__NR_openat, dirfd, pathname, flags, mode); +} + long __clone(int (*fn)(void *), uintcap_t stack, int flags, void *arg, pid_t *parent_tid, void *tls, pid_t *child_tid);
@@ -174,8 +185,49 @@ static inline int munmap(void *addr, size_t length) return syscall(__NR_munmap, addr, length); }
+static inline int madvise(void *addr, size_t length, int advise) +{ + return syscall(__NR_madvise, addr, length, advise); +} + +static inline int mincore(void *addr, size_t length, unsigned char *vec) +{ + return syscall(__NR_mincore, addr, length, vec); +} + +static inline int mlock(const void *addr, size_t len) +{ + return syscall(__NR_mlock, addr, len); +} + +static inline int mlock2(const void *addr, size_t len, unsigned int flags) +{ + return syscall(__NR_mlock2, addr, len, flags); +} + +static inline int munlock(const void *addr, size_t len) +{ + return syscall(__NR_munlock, addr, len); +} + +static inline int msync(void *addr, size_t length, int flags) +{ + return syscall(__NR_msync, addr, length, flags); +} + +static inline int mprotect(void *addr, size_t length, int prot) +{ + return syscall(__NR_mprotect, addr, length, prot); +} + +static inline void *mremap(void *old_address, size_t old_size, size_t new_size, + int flags, void *new_address) +{ + return (void *)syscall(__NR_mremap, old_address, old_size, new_size, flags, new_address); +} + static inline void *mmap_verified(void *addr, size_t length, int prot, int flags, - int fd, int offset, unsigned int perms) + int fd, int offset, unsigned int perms) { void *__addr = mmap(addr, length, prot, flags, fd, offset);
@@ -200,6 +252,11 @@ static inline void *mmap_verified(void *addr, size_t length, int prot, int flags return NULL; }
+static inline int brk(void *addr) +{ + return syscall(__NR_brk, addr); +} + static inline int close(int fd) { return syscall(__NR_close, fd); diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 5d259c336f55..2dd4ccdb0d2a 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -71,11 +71,11 @@ void syscall_mmap2(void) int retval;
/* create a sample file to map onto with mmap */ - fd = syscall(__NR_openat, 0, sample_file, O_RDWR | O_CREAT, FILE_PERM); + fd = openat(0, sample_file, O_RDWR | O_CREAT, FILE_PERM);
ASSERT_GE(fd, 0);
- retval = syscall(__NR_lseek, fd, MMAP_SIZE, SEEK_SET); + retval = lseek(fd, MMAP_SIZE, SEEK_SET); ASSERT_EQ(retval, MMAP_SIZE);
/* attempt to write arbitrary data to file */ @@ -92,18 +92,17 @@ void syscall_mmap2(void) PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
/* Attempt to change bounds of memory mapping, shrink by factor of 2 */ - addr = (void *)syscall(__NR_mremap, addr, MMAP_SIZE, - MMAP_SIZE_REDUCED, 0, 0); + addr = mremap(addr, MMAP_SIZE, MMAP_SIZE_REDUCED, 0, 0);
ASSERT_FALSE(IS_ERR_VALUE(addr)); /* advise kernel about how to handle paging of mapped memory.*/ - retval = syscall(__NR_madvise, addr, MMAP_SIZE_REDUCED, MADV_WILLNEED); + retval = madvise(addr, MMAP_SIZE_REDUCED, MADV_WILLNEED); ASSERT_EQ(retval, 0);
EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE_REDUCED, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); /* An attempt to change permissions to RO */ - retval = syscall(__NR_mprotect, addr, MMAP_SIZE_REDUCED, PROT_READ); + retval = mprotect(addr, MMAP_SIZE_REDUCED, PROT_READ); ASSERT_EQ(retval, 0); /* Write permission should be revoked - verify mode only */ /* To be extended when signals are fully supported */
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
As Morello kselftests are standalone and don't make use of standard C libraries at the moment, a get_pagesize() function has been added to retrieve the page size of a system at runtime. The morello_auxv and initial_data structs originally defined in bootstrap.c, have been moved to freestanding.h to facilitate their access from multiple files.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/bootstrap.c | 13 ------------- .../selftests/arm64/morello/freestanding.c | 15 +++++++++++++++ .../selftests/arm64/morello/freestanding.h | 15 +++++++++++++++ tools/testing/selftests/arm64/morello/mmap.c | 7 ++++++- 4 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/bootstrap.c b/tools/testing/selftests/arm64/morello/bootstrap.c index d594fcb3fade..e9075f595b85 100644 --- a/tools/testing/selftests/arm64/morello/bootstrap.c +++ b/tools/testing/selftests/arm64/morello/bootstrap.c @@ -37,19 +37,6 @@ #define ASSERT_CAP_EQ(exp, seen) \ ASSERT_TRUE(__builtin_cheri_equal_exact(exp, seen))
-struct morello_auxv { - long a_type; - long _padding; - uintcap_t a_val; -}; - -struct initial_data { - int argc; - char **argv; - char **envp; - struct morello_auxv *auxv; -}; - static struct initial_data reg_data;
int clear_child_tid; diff --git a/tools/testing/selftests/arm64/morello/freestanding.c b/tools/testing/selftests/arm64/morello/freestanding.c index 45c0fa8b0914..78b9c077a233 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.c +++ b/tools/testing/selftests/arm64/morello/freestanding.c @@ -6,6 +6,7 @@ #include <stdbool.h>
#include <linux/errno.h> +#include <linux/auxvec.h>
#include "freestanding.h"
@@ -93,6 +94,20 @@ static ssize_t __write_all(const char *str, size_t len) return written; }
+unsigned long get_pagesize(struct morello_auxv *auxv) +{ + unsigned long page_size = 0; + + while (auxv->a_type != AT_NULL) { + if (auxv->a_type == AT_PAGESZ) { + page_size = auxv->a_val; + break; + } + ++auxv; + } + return page_size; +} + /* * formats supported: %d, %x, %s, %p, * modifiers l/z/u are accepted and ignored. To compensate, values are always diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 901521bde55d..ed85165dbb70 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -43,6 +43,21 @@ struct __test_meta { int message; };
+struct morello_auxv { + long a_type; + long _padding; + uintcap_t a_val; +}; + +struct initial_data { + int argc; + char **argv; + char **envp; + struct morello_auxv *auxv; +}; + +unsigned long get_pagesize(struct morello_auxv *auxv); + void install_kernel_stack(void); uintcap_t __syscall(uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t);
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 2dd4ccdb0d2a..89647ce8bec3 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -19,6 +19,9 @@ #define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02
+#define __unused __attribute__((unused)) + +static unsigned long pagesize;
static inline int probe_mem_range(void *addr, size_t size, int mode) { @@ -127,8 +130,10 @@ TEST(test_syscall_mmap2) syscall_mmap2(); }
-int main(void) +int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { + pagesize = get_pagesize(auxv); + test_syscall_mmap(); test_syscall_mmap2(); return 0;
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
As Morello kselftests are standalone and don't make use of standard C libraries at the moment, a get_pagesize() function has been added to retrieve the page size of a system at runtime. The morello_auxv and initial_data structs originally defined in bootstrap.c, have been moved to freestanding.h to facilitate their access from multiple files.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
tools/testing/selftests/arm64/morello/bootstrap.c | 13 ------------- .../selftests/arm64/morello/freestanding.c | 15 +++++++++++++++ .../selftests/arm64/morello/freestanding.h | 15 +++++++++++++++ tools/testing/selftests/arm64/morello/mmap.c | 7 ++++++- 4 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/bootstrap.c b/tools/testing/selftests/arm64/morello/bootstrap.c index d594fcb3fade..e9075f595b85 100644 --- a/tools/testing/selftests/arm64/morello/bootstrap.c +++ b/tools/testing/selftests/arm64/morello/bootstrap.c @@ -37,19 +37,6 @@ #define ASSERT_CAP_EQ(exp, seen) \ ASSERT_TRUE(__builtin_cheri_equal_exact(exp, seen)) -struct morello_auxv {
- long a_type;
- long _padding;
- uintcap_t a_val;
-};
-struct initial_data {
- int argc;
- char **argv;
- char **envp;
- struct morello_auxv *auxv;
-};
This struct doesn't need to be moved any more (the commit message should be updated as well).
static struct initial_data reg_data; int clear_child_tid; diff --git a/tools/testing/selftests/arm64/morello/freestanding.c b/tools/testing/selftests/arm64/morello/freestanding.c index 45c0fa8b0914..78b9c077a233 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.c +++ b/tools/testing/selftests/arm64/morello/freestanding.c @@ -6,6 +6,7 @@ #include <stdbool.h> #include <linux/errno.h> +#include <linux/auxvec.h> #include "freestanding.h" @@ -93,6 +94,20 @@ static ssize_t __write_all(const char *str, size_t len) return written; } +unsigned long get_pagesize(struct morello_auxv *auxv) +{
- unsigned long page_size = 0;
- while (auxv->a_type != AT_NULL) {
if (auxv->a_type == AT_PAGESZ) {page_size = auxv->a_val;break;}++auxv;- }
- return page_size;
+}
/*
- formats supported: %d, %x, %s, %p,
- modifiers l/z/u are accepted and ignored. To compensate, values are always
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 901521bde55d..ed85165dbb70 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -43,6 +43,21 @@ struct __test_meta { int message; }; +struct morello_auxv {
- long a_type;
- long _padding;
- uintcap_t a_val;
+};
+struct initial_data {
- int argc;
- char **argv;
- char **envp;
- struct morello_auxv *auxv;
+};
+unsigned long get_pagesize(struct morello_auxv *auxv);
void install_kernel_stack(void); uintcap_t __syscall(uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t, uintcap_t); diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 2dd4ccdb0d2a..89647ce8bec3 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -19,6 +19,9 @@ #define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02 +#define __unused __attribute__((unused))
Let's call it __maybe_unused to match what the kernel (and other kselftests) use (see linux/compiler_attributes.h). Also might as well add it to freestanding.h.
+static unsigned long pagesize; static inline int probe_mem_range(void *addr, size_t size, int mode) { @@ -127,8 +130,10 @@ TEST(test_syscall_mmap2) syscall_mmap2(); } -int main(void) +int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv)
It's more common to add attributes after the declaration, e.g. int argc __maybe_unused.
Kevin
{
- pagesize = get_pagesize(auxv);
- test_syscall_mmap(); test_syscall_mmap2(); return 0;
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Helper functions for syscall_mmap and syscall_mmap2 have been removed and the code is rewritten directly in testcase functions. The pointer variable "addr" is renamed to "ptr" in order to avoid confusion when manipulating both addresses and pointers.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 54 ++++++++------------ 1 file changed, 21 insertions(+), 33 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 89647ce8bec3..08a209ffc6de 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -23,9 +23,9 @@
static unsigned long pagesize;
-static inline int probe_mem_range(void *addr, size_t size, int mode) +static inline int probe_mem_range(void *ptr, size_t size, int mode) { - unsigned int *p = (unsigned int *)addr; + unsigned int *p = (unsigned int *)ptr; size_t probe_size = size / sizeof(unsigned int);
if (mode & PROBE_MODE_TOUCH) { @@ -43,33 +43,31 @@ static inline int probe_mem_range(void *addr, size_t size, int mode) /* Simple test to check our ability to create a new anonymous mapping * in the virtual address space of the calling process */ -static inline __attribute__((always_inline)) -void syscall_mmap(void) +TEST(test_syscall_mmap) {
- void *addr = mmap_verified(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, + void *ptr = mmap_verified(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0, CAP_LOAD_PERMS | CAP_STORE_PERMS);
- ASSERT_NE(addr, NULL); + ASSERT_NE(ptr, NULL);
- EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE, - PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)) { + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)) { TH_LOG("Failed on probing allocated mem range\n"); } - EXPECT_EQ(0, munmap(addr, MMAP_SIZE)); + EXPECT_EQ(0, munmap(ptr, MMAP_SIZE)); }
/* test mmap providing it with a file descriptor, testing related * functionality */ -static inline __attribute__((always_inline)) -void syscall_mmap2(void) +TEST(test_syscall_mmap2) { const char *msg = "foo"; unsigned int msg_len = sizeof(msg); /* No need for the terminator */ const char *sample_file = "/limbo.dat"; - void *addr; + void *ptr; int fd; int retval;
@@ -85,51 +83,41 @@ void syscall_mmap2(void) retval = write(fd, msg, msg_len); ASSERT_EQ(retval, (int)msg_len);
- addr = mmap_verified(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, + ptr = mmap_verified(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0, CAP_LOAD_PERMS | CAP_STORE_PERMS);
- EXPECT_NE(addr, NULL) + EXPECT_NE(ptr, NULL) goto clean_up;
- EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE, - PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
/* Attempt to change bounds of memory mapping, shrink by factor of 2 */ - addr = mremap(addr, MMAP_SIZE, MMAP_SIZE_REDUCED, 0, 0); + ptr = mremap(ptr, MMAP_SIZE, MMAP_SIZE_REDUCED, 0, 0);
- ASSERT_FALSE(IS_ERR_VALUE(addr)); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); /* advise kernel about how to handle paging of mapped memory.*/ - retval = madvise(addr, MMAP_SIZE_REDUCED, MADV_WILLNEED); + retval = madvise(ptr, MMAP_SIZE_REDUCED, MADV_WILLNEED); ASSERT_EQ(retval, 0);
- EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE_REDUCED, + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE_REDUCED, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); /* An attempt to change permissions to RO */ - retval = mprotect(addr, MMAP_SIZE_REDUCED, PROT_READ); + retval = mprotect(ptr, MMAP_SIZE_REDUCED, PROT_READ); ASSERT_EQ(retval, 0); /* Write permission should be revoked - verify mode only */ /* To be extended when signals are fully supported */ - EXPECT_EQ(0, probe_mem_range(addr, MMAP_SIZE_REDUCED, PROBE_MODE_VERIFY)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE_REDUCED, PROBE_MODE_VERIFY));
clean_up: /* do unmap */ - munmap(addr, MMAP_SIZE_REDUCED); + munmap(ptr, MMAP_SIZE_REDUCED); ASSERT_EQ(retval, 0);
/* do file close */ close(fd); }
-TEST(test_syscall_mmap) -{ - syscall_mmap(); -} - -TEST(test_syscall_mmap2) -{ - syscall_mmap2(); -} - int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv);
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
The mmap() system call is expected to fail with -EOPNOTSUPP when the MAP_GROWSDOWN flag is passed. A testcase to verify this behaviour has been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 08a209ffc6de..ea6ea88f1294 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -118,11 +118,23 @@ TEST(test_syscall_mmap2) close(fd); }
+/* test to verify mmap() behaviour when MAP_GROWSDOWN flag is specified */ +TEST(test_map_growsdown) +{ + void *ptr; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN; + + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + EXPECT_EQ((unsigned long)ptr, (unsigned long)-EOPNOTSUPP); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv);
test_syscall_mmap(); test_syscall_mmap2(); + test_map_growsdown(); return 0; }
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Only valid owning capabilities are allowed to manage memory mappings. Passing a capability with it's tag bit cleared will result in failure of the syscall. Tests to verify this behaviour have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 93 ++++++++++++++++++++ 1 file changed, 93 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index ea6ea88f1294..01ca77593c94 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -129,6 +129,98 @@ TEST(test_map_growsdown) EXPECT_EQ((unsigned long)ptr, (unsigned long)-EOPNOTSUPP); }
+/* test to validate parameters passed to address space management syscalls */ +TEST(test_validity_tag_check) +{ + void *ptr, *new_ptr; + int retval; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + unsigned char vec[MMAP_SIZE / pagesize]; + + /* passing invalid capability to mmap() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + new_ptr = mmap(cheri_tag_clear(ptr), MMAP_SIZE_REDUCED, prot, + flags | MAP_FIXED, -1, 0); + EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to munmap() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(cheri_tag_clear(ptr), MMAP_SIZE); + EXPECT_EQ(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to mremap() */ + ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + new_ptr = mremap(cheri_tag_clear(ptr), MMAP_SIZE_REDUCED, MMAP_SIZE, + MREMAP_MAYMOVE, 0); + EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to mprotect() */ + ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(prot) | PROT_READ, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mprotect(cheri_tag_clear(ptr), MMAP_SIZE, PROT_WRITE); + EXPECT_EQ(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* as the remaining syscalls are expected to fail in a similar manner, + * have a common mapping. + */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + /* passing invalid capability to madvise() */ + retval = madvise(cheri_tag_clear(ptr), MMAP_SIZE, MADV_WILLNEED); + EXPECT_EQ(retval, -EINVAL); + + /* passing invalid capability to mincore() */ + retval = mincore(cheri_tag_clear(ptr), MMAP_SIZE, vec); + EXPECT_EQ(retval, -EINVAL); + + /* passing invalid capability to mlock() */ + retval = mlock(cheri_tag_clear(ptr), MMAP_SIZE); + EXPECT_EQ(retval, -EINVAL); + + /* passing invalid capability to mlock2() */ + retval = mlock2(cheri_tag_clear(ptr), MMAP_SIZE, MLOCK_ONFAULT); + EXPECT_EQ(retval, -EINVAL); + + /* passing invalid capability to munlock() */ + EXPECT_EQ(0, mlock(ptr, MMAP_SIZE_REDUCED)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE_REDUCED, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munlock(cheri_tag_clear(ptr), MMAP_SIZE_REDUCED); + EXPECT_EQ(retval, -EINVAL); + + retval = munlock(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to msync() */ + retval = msync(cheri_tag_clear(ptr), MMAP_SIZE, MS_SYNC); + EXPECT_EQ(retval, -EINVAL); + + /* unmap the common mapping */ + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -136,5 +228,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_syscall_mmap(); test_syscall_mmap2(); test_map_growsdown(); + test_validity_tag_check(); return 0; }
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Only valid owning capabilities are allowed to manage memory mappings. Passing a capability with it's tag bit cleared will result in failure
s/it's/its/
of the syscall. Tests to verify this behaviour have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
tools/testing/selftests/arm64/morello/mmap.c | 93 ++++++++++++++++++++ 1 file changed, 93 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index ea6ea88f1294..01ca77593c94 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -129,6 +129,98 @@ TEST(test_map_growsdown) EXPECT_EQ((unsigned long)ptr, (unsigned long)-EOPNOTSUPP); } +/* test to validate parameters passed to address space management syscalls */
That comment should be updated, as this test is only about tag check. Same comment in patch 9.
Kevin
[...]
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Overwriting part of a mapping within an existing reservation is allowed with the use of the MAP_FIXED flag. Whereas any attempt to write beyond the bounds of the existing reservation would cause the syscall to fail.
Address space management syscalls that manipulate a given mapping are restricted to the range owned by the capability. Any attempt to manage mappings beyond this range will result in failure of the syscall.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 85 ++++++++++++++++++++ 1 file changed, 85 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 01ca77593c94..daf69633e182 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -221,6 +221,90 @@ TEST(test_validity_tag_check) ASSERT_EQ(retval, 0); }
+/* test to verify address space management syscall behaviour when capability + * range is modified + */ +TEST(test_range_check) +{ + void *ptr, *reduced_bound_ptr, *ret; + int retval; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + unsigned char vec[MMAP_SIZE / pagesize]; + + /* mapping a smaller range at prev mmap ptr in a subsequent mmap() + * call without first unmapping + */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + ret = mmap(ptr, MMAP_SIZE_REDUCED, prot, flags | MAP_FIXED, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ret)); + EXPECT_EQ(0, probe_mem_range(ret, MMAP_SIZE_REDUCED, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* mapping a larger range at prev mmap ptr in a subsequent mmap() + * call without first unmapping + */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + ret = mmap(ptr, 2 * MMAP_SIZE, prot, flags | MAP_FIXED, -1, 0); + EXPECT_EQ((unsigned long)ret, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* as the following syscalls are expected to fail in a similar manner, + * have a common mapping and reduced bound pointer. + */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + reduced_bound_ptr = cheri_bounds_set(ptr, MMAP_SIZE_REDUCED); + + /* negative munmap() range test */ + retval = munmap(reduced_bound_ptr, MMAP_SIZE); + EXPECT_EQ(retval, -EINVAL); + + /* negative mincore() range test */ + retval = mincore(reduced_bound_ptr, MMAP_SIZE, vec); + EXPECT_EQ(retval, -EINVAL); + + /* negative mlock() range test */ + retval = mlock(reduced_bound_ptr, MMAP_SIZE); + EXPECT_EQ(retval, -EINVAL); + + /* negative munlock() range test */ + EXPECT_EQ(0, mlock2(ptr, MMAP_SIZE, MLOCK_ONFAULT)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munlock(reduced_bound_ptr, MMAP_SIZE); + EXPECT_EQ(retval, -EINVAL); + + retval = munlock(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* negative msync() range test */ + retval = msync(reduced_bound_ptr, MMAP_SIZE, MS_SYNC); + EXPECT_EQ(retval, -EINVAL); + + /* release the common mapping */ + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* negative madvise() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, MAP_SHARED | MAP_ANONYMOUS, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + reduced_bound_ptr = cheri_bounds_set(ptr, MMAP_SIZE_REDUCED); + + retval = madvise(reduced_bound_ptr, MMAP_SIZE, MADV_NORMAL); + EXPECT_EQ(retval, -EINVAL); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -229,5 +313,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_syscall_mmap2(); test_map_growsdown(); test_validity_tag_check(); + test_range_check(); return 0; }
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Overwriting part of a mapping within an existing reservation is allowed with the use of the MAP_FIXED flag. Whereas any attempt to write beyond the bounds of the existing reservation would cause the syscall to fail.
AFAICT these tests are purely abound capability bounds check, not reservation bounds.
[...]
- /* negative madvise() range test */
- ptr = mmap(NULL, MMAP_SIZE, prot, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
Can't we use the same (private) mapping to test madvise()?
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- reduced_bound_ptr = cheri_bounds_set(ptr, MMAP_SIZE_REDUCED);
- retval = madvise(reduced_bound_ptr, MMAP_SIZE, MADV_NORMAL);
- EXPECT_EQ(retval, -EINVAL);
+}
[...]
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Reservations are contiguous ranges of virtual addresses that exactly match the bounds of an owning capability. When an owning capability is passed to a syscall, it's bounds are first verified against the existing reservation. If the bounds of a null-derived capability are found to overlap with any existing reservation, the syscall fails with an -ERESERVATION error code. A partial unmap within a particular reservation still allows the rest of the region to be accessible. Tests to verify the same have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index daf69633e182..727937b49c11 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -305,6 +305,44 @@ TEST(test_range_check) EXPECT_EQ(retval, -EINVAL); }
+/* test to verify mmap() behaviour when capability bounds are modified */ +TEST(test_mmap_bounds_check) +{ + void *addr, *ptr, *new_ptr; + size_t size; + int retval; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + + /* test to verify rest of reservation region is accessible after a partial + * unmap + */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + retval = munmap(ptr, pagesize); + ASSERT_EQ(retval, 0); + ptr = ptr + pagesize; + size = MMAP_SIZE - pagesize; + EXPECT_EQ(0, probe_mem_range(ptr, size, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(ptr, size); + ASSERT_EQ(retval, 0); + + /* null-derived ptr overlaps with an existing resservation */ + addr = (void *)(uintptr_t)(pagesize * 8); + ptr = mmap(addr, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + new_ptr = mmap(addr + MMAP_SIZE_REDUCED, MMAP_SIZE, prot, + flags | MAP_FIXED, -1, 0); + EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-ERESERVATION); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -314,5 +352,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_map_growsdown(); test_validity_tag_check(); test_range_check(); + test_mmap_bounds_check(); return 0; }
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Reservations are contiguous ranges of virtual addresses that exactly match the bounds of an owning capability. When an owning capability is passed to a syscall, it's bounds are first verified against the existing
s/it's/its/
reservation. If the bounds of a null-derived capability are found to
A null-derived capability has no bounds, maybe this is about the reservation that would be created?
overlap with any existing reservation, the syscall fails with an -ERESERVATION error code. A partial unmap within a particular reservation still allows the rest of the region to be accessible. Tests to verify the same have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
tools/testing/selftests/arm64/morello/mmap.c | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index daf69633e182..727937b49c11 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -305,6 +305,44 @@ TEST(test_range_check) EXPECT_EQ(retval, -EINVAL); } +/* test to verify mmap() behaviour when capability bounds are modified */ +TEST(test_mmap_bounds_check)
The name / comment are rather confusing as this is not about capability bounds check, but rather reservations themselves. Same comment regarding patch 8.
+{
- void *addr, *ptr, *new_ptr;
To avoid address / pointer confusion, I think it would be better to make addr a ptraddr_t, and then cast it as needed (when passing it to mmap()). Same comment in patch 9.
- size_t size;
- int retval;
- int prot = PROT_READ | PROT_WRITE;
- int flags = MAP_PRIVATE | MAP_ANONYMOUS;
- /* test to verify rest of reservation region is accessible after a partial
* unmap*/- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = munmap(ptr, pagesize);
- ASSERT_EQ(retval, 0);
- ptr = ptr + pagesize;
- size = MMAP_SIZE - pagesize;
- EXPECT_EQ(0, probe_mem_range(ptr, size,
PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));- retval = munmap(ptr, size);
- ASSERT_EQ(retval, 0);
- /* null-derived ptr overlaps with an existing resservation */
s/resservation/reservation/
- addr = (void *)(uintptr_t)(pagesize * 8);
This value definitely warrants a comment, as it looks pretty arbitrary otherwise.
Kevin
- ptr = mmap(addr, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- new_ptr = mmap(addr + MMAP_SIZE_REDUCED, MMAP_SIZE, prot,
flags | MAP_FIXED, -1, 0);- EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-ERESERVATION);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
+}
int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -314,5 +352,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_map_growsdown(); test_validity_tag_check(); test_range_check();
- test_mmap_bounds_check(); return 0;
}
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Attempting to remap a range larger than what is owned by the capability triggers a -EINVAL error. Additionally, mappings that have to be moved in order to satisfy the new constraints, expect the MREMAP_MAYMOVE flag to be specified. Failure to do so triggers the -ENOMEM error. Within the bounds of a reservation, mremap() can be used to grow the mappings in-place. Tests to verify this behaviour have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 57 ++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 727937b49c11..610ba12a35ff 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -343,6 +343,62 @@ TEST(test_mmap_bounds_check) ASSERT_EQ(retval, 0); }
+/* test to verify mremap() behaviour when capability bounds are modified */ +TEST(test_mremap_bounds_check) +{ + void *addr, *ptr, *new_ptr; + int retval; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + + /* moving a mapping with MREMAP_MAYMOVE flag specified */ + ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, MREMAP_MAYMOVE, NULL); + ASSERT_FALSE(IS_ERR_VALUE(new_ptr)); + ASSERT_NE((unsigned long)ptr, (unsigned long)new_ptr); + EXPECT_EQ(0, probe_mem_range(new_ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(new_ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* moving a mapping without MREMAP_MAYMOVE flag triggers an ENOMEM error */ + ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, 0, 0); + EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-ENOMEM); + + retval = munmap(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + /* attempt to resize a mapping range greater than what the capability owns */ + ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + new_ptr = mremap(ptr, MMAP_SIZE, MMAP_SIZE, MREMAP_MAYMOVE, 0); + EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + /* attempt to grow mappings in-place */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + addr = (void *)(uintptr_t)(ptr + MMAP_SIZE_REDUCED); + retval = munmap(addr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, 0, 0); + ASSERT_FALSE(IS_ERR_VALUE(new_ptr)); + + retval = munmap(new_ptr, MMAP_SIZE); + EXPECT_EQ(retval, 0); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -353,5 +409,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_validity_tag_check(); test_range_check(); test_mmap_bounds_check(); + test_mremap_bounds_check(); return 0; }
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
Attempting to remap a range larger than what is owned by the capability triggers a -EINVAL error. Additionally, mappings that have to be moved in order to satisfy the new constraints, expect the MREMAP_MAYMOVE flag to be specified. Failure to do so triggers the -ENOMEM error. Within the bounds of a reservation, mremap() can be used to grow the mappings in-place. Tests to verify this behaviour have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
tools/testing/selftests/arm64/morello/mmap.c | 57 ++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 727937b49c11..610ba12a35ff 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -343,6 +343,62 @@ TEST(test_mmap_bounds_check) ASSERT_EQ(retval, 0); } +/* test to verify mremap() behaviour when capability bounds are modified */ +TEST(test_mremap_bounds_check) +{
- void *addr, *ptr, *new_ptr;
- int retval;
- int prot = PROT_READ | PROT_WRITE;
- int flags = MAP_PRIVATE | MAP_ANONYMOUS;
- /* moving a mapping with MREMAP_MAYMOVE flag specified */
- ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, MREMAP_MAYMOVE, NULL);
- ASSERT_FALSE(IS_ERR_VALUE(new_ptr));
- ASSERT_NE((unsigned long)ptr, (unsigned long)new_ptr);
No need for the casts.
- EXPECT_EQ(0, probe_mem_range(new_ptr, MMAP_SIZE,
PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));- retval = munmap(new_ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* moving a mapping without MREMAP_MAYMOVE flag triggers an ENOMEM error */
This is a bit confusing, because as such the mremap() call below doesn't ask for the mapping to be moved. It is only because of reservation restrictions that it would be necessary to move it. Maybe "expanding a mapping"? That would also make sense in the comment above.
- ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, 0, 0);
- EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-ENOMEM);
- retval = munmap(ptr, MMAP_SIZE_REDUCED);
- ASSERT_EQ(retval, 0);
- /* attempt to resize a mapping range greater than what the capability owns */
- ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- new_ptr = mremap(ptr, MMAP_SIZE, MMAP_SIZE, MREMAP_MAYMOVE, 0);
- EXPECT_EQ((unsigned long)new_ptr, (unsigned long)-EINVAL);
- retval = munmap(ptr, MMAP_SIZE_REDUCED);
- ASSERT_EQ(retval, 0);
- /* attempt to grow mappings in-place */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- addr = (void *)(uintptr_t)(ptr + MMAP_SIZE_REDUCED);
It doesn't look like you need the temporary and casts, just pass ptr + MMAP_SIZE_REDUCED to munmap() directly?
Kevin
- retval = munmap(addr, MMAP_SIZE_REDUCED);
- ASSERT_EQ(retval, 0);
- new_ptr = mremap(ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, 0, 0);
- ASSERT_FALSE(IS_ERR_VALUE(new_ptr));
- retval = munmap(new_ptr, MMAP_SIZE);
- EXPECT_EQ(retval, 0);
+}
int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -353,5 +409,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_validity_tag_check(); test_range_check(); test_mmap_bounds_check();
- test_mremap_bounds_check(); return 0;
}
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
When a capability is created it is assigned the maximum permissions it may ever have, by passing PROT_MAX(max_prot) as one of the prot flags. Any attempt to increase the permissions beyond this would result in failure of the syscall. An owning capability returned by mmap() does not include LoadCap and StoreCap permissions if the mapping is shared. A check to verify the same has been added.
mremap() doesn't take a prot argument and retains the permissions of the original capability. If the permissions of the new capability do not match the set of permissions of the old capabaility, the syscall fails with an -EINVAL error code. Tests to verify the above behaviour have been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 145 +++++++++++++++++++ 1 file changed, 145 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 610ba12a35ff..bbff1d2b1d31 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -15,6 +15,7 @@ #define MMAP_SIZE ((1ULL << 16) << 1) /* 64k x 2 */ #define MMAP_SIZE_REDUCED (MMAP_SIZE >> 1) #define FILE_PERM 0666 +#define PROT_ALL (PROT_READ | PROT_WRITE | PROT_EXEC)
#define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02 @@ -399,6 +400,149 @@ TEST(test_mremap_bounds_check) EXPECT_EQ(retval, 0); }
+/* test to verify mremap() behaviour when permissions are modified */ +TEST(test_permissions) +{ + void *addr, *ptr, *old_ptr, *new_ptr, *ret; + int flags, retval; + int prot, max_prot; + size_t perms, old_perms, new_perms; + + /* increase permission beyond the maximum prot specified for the mapping */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + max_prot = PROT_READ | PROT_WRITE; + prot = PROT_READ; + + ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(max_prot) | prot, + flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + retval = mprotect(ptr, MMAP_SIZE, PROT_EXEC); + ASSERT_EQ(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* max_prot has fewer permissions than prot */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + max_prot = PROT_WRITE | PROT_EXEC; + prot = PROT_ALL; + + ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(max_prot) | prot, flags, -1, 0); + EXPECT_EQ((unsigned long)ptr, (unsigned long)-EINVAL); + + /* max_prot has more permissions than prot */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + max_prot = PROT_ALL; + prot = PROT_READ | PROT_EXEC; + + ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(max_prot) | prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + retval = mprotect(ptr, MMAP_SIZE, PROT_WRITE); + ASSERT_EQ(retval, 0); + + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* repeat positive max_prot test with fixed address */ + addr = (void *)(uintptr_t)(pagesize * 16); + flags = MAP_PRIVATE | MAP_ANONYMOUS; + max_prot = PROT_ALL; + prot = PROT_READ | PROT_EXEC; + + ptr = mmap(addr, MMAP_SIZE, PROT_MAX(max_prot) | prot, + flags | MAP_FIXED, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + retval = mprotect(ptr, MMAP_SIZE, PROT_WRITE); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* LoadCap and StoreCap permissions must not be given to a shared mapping */ + flags = MAP_SHARED | MAP_ANONYMOUS; + prot = PROT_READ | PROT_WRITE; + + ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(prot) | PROT_READ, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + + perms = cheri_perms_get(ptr); + retval = ((perms & CHERI_PERM_LOAD) && (perms & CHERI_PERM_STORE_CAP)); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* permissions of capability returned by mremap must match the + * permissions returned by the original mapping + */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + prot = PROT_READ | PROT_WRITE; + + old_ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(old_ptr)); + + old_perms = cheri_perms_get(old_ptr); + new_ptr = mremap(old_ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, + MREMAP_MAYMOVE, 0); + ASSERT_FALSE(IS_ERR_VALUE(new_ptr)); + + new_perms = cheri_perms_get(new_ptr); + ASSERT_EQ(old_perms, new_perms); + EXPECT_EQ(0, probe_mem_range(new_ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(new_ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* remapping to a new_ptr having reduced permissions from old_ptr */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + prot = PROT_READ | PROT_WRITE; + + old_ptr = mmap(NULL, MMAP_SIZE_REDUCED, PROT_MAX(prot | PROT_EXEC) | + prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(old_ptr)); + + new_ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(prot) | PROT_READ, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(new_ptr)); + + ret = mremap(old_ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, + MREMAP_MAYMOVE | MREMAP_FIXED, new_ptr); + ASSERT_FALSE(IS_ERR_VALUE(ret)); + EXPECT_EQ(0, probe_mem_range(new_ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(ret, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* remapping to new_ptr having increased permissions from old_ptr */ + flags = MAP_PRIVATE | MAP_ANONYMOUS; + prot = PROT_READ | PROT_WRITE; + + old_ptr = mmap(NULL, MMAP_SIZE_REDUCED, PROT_MAX(prot) | PROT_READ, + flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(old_ptr)); + + new_ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(prot | PROT_EXEC) | prot, + flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(new_ptr)); + + ret = mremap(old_ptr, MMAP_SIZE_REDUCED, MMAP_SIZE, + MREMAP_MAYMOVE | MREMAP_FIXED, new_ptr); + EXPECT_EQ((unsigned long)ret, (unsigned long)-EINVAL); + + retval = munmap(new_ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + retval = munmap(old_ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -410,5 +554,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_range_check(); test_mmap_bounds_check(); test_mremap_bounds_check(); + test_permissions(); return 0; }
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
When a capability is created it is assigned the maximum permissions it may ever have, by passing PROT_MAX(max_prot) as one of the prot flags. Any attempt to increase the permissions beyond this would result in failure of the syscall. An owning capability returned by mmap() does not include LoadCap and StoreCap permissions if the mapping is shared. A check to verify the same has been added.
mremap() doesn't take a prot argument and retains the permissions of the original capability. If the permissions of the new capability do not match the set of permissions of the old capabaility, the syscall fails with an -EINVAL error code.
Isn't that only true if the new set of permissions is larger than the old set?
[...]
- /* repeat positive max_prot test with fixed address */
- addr = (void *)(uintptr_t)(pagesize * 16);
It's not clear why a different value is used here compared to patch 7. It probably makes sense to define a constant shared between the tests.
- flags = MAP_PRIVATE | MAP_ANONYMOUS;
- max_prot = PROT_ALL;
- prot = PROT_READ | PROT_EXEC;
- ptr = mmap(addr, MMAP_SIZE, PROT_MAX(max_prot) | prot,
flags | MAP_FIXED, -1, 0);- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = mprotect(ptr, MMAP_SIZE, PROT_WRITE);
- ASSERT_EQ(retval, 0);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* LoadCap and StoreCap permissions must not be given to a shared mapping */
- flags = MAP_SHARED | MAP_ANONYMOUS;
- prot = PROT_READ | PROT_WRITE;
- ptr = mmap(NULL, MMAP_SIZE, PROT_MAX(prot) | PROT_READ, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- perms = cheri_perms_get(ptr);
- retval = ((perms & CHERI_PERM_LOAD) && (perms & CHERI_PERM_STORE_CAP));
I must have missed this in v4. Right now this checks that either Load or StoreCap is absent, which is probably not the point. I guess (perms & (CHERI_PERM_LOAD_CAP | CHERI_PERM_STORE_CAP))? That could be directly in the EXPECT_EQ().
- ASSERT_EQ(retval, 0);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* permissions of capability returned by mremap must match the
* permissions returned by the original mapping*/- flags = MAP_PRIVATE | MAP_ANONYMOUS;
- prot = PROT_READ | PROT_WRITE;
- old_ptr = mmap(NULL, MMAP_SIZE_REDUCED, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(old_ptr));
- old_perms = cheri_perms_get(old_ptr);
- new_ptr = mremap(old_ptr, MMAP_SIZE_REDUCED, MMAP_SIZE,
MREMAP_MAYMOVE, 0);- ASSERT_FALSE(IS_ERR_VALUE(new_ptr));
- new_perms = cheri_perms_get(new_ptr);
- ASSERT_EQ(old_perms, new_perms);
old_ptr remains valid, so we can directly use cheri_perms_get() here without temporaries.
[...]
From: Chaitanya S Prakash chaitanyas.prakash@arm.com
As the mechanism of brk() depends on implicit address space reservation by moving the program break, it is unfavourable to the capability model. Hence an assumption is made that brk() is unnecessary and allocators making use of it can use mmap() instead. If used, it returns -ENOSYS. A test to verify this behaviour has been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index bbff1d2b1d31..cb40bb88b1dd 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -543,6 +543,15 @@ TEST(test_permissions) ASSERT_EQ(retval, 0); }
+/* test to verify that using brk() results syscall failure */ +TEST(test_brk_check) +{ + int retval; + + retval = brk(NULL); + EXPECT_EQ(retval, -ENOSYS); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -555,5 +564,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_mmap_bounds_check(); test_mremap_bounds_check(); test_permissions(); + test_brk_check(); return 0; }
From: Amit Daniel Kachhap amit.kachhap@arm.com
Morello uses a compressed capability format which makes it difficult to represent bounds with arbitrary precision. As the corresponding address range of a given memory mapping may not be exactly representable as valid capability bounds, a test to verify that the PCuABI kernel is able to mmap/munmap those addresses has been added.
Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com Signed-off-by: Chaitanya S Prakash ChaitanyaS.Prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index cb40bb88b1dd..50e1c8313ecd 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -552,6 +552,43 @@ TEST(test_brk_check) EXPECT_EQ(retval, -ENOSYS); }
+/* test to verify the Cheri unrepresentable address/length */ +TEST(test_cheri_unrepresentability) +{ + void *ptr1, *ptr2; + int retval; + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS; + size_t len; + unsigned long inc_page = 0; + + /* Use pageshift 16 for 64K pages so as to use as mmap fixed address */ + unsigned long pageshift = 16; + + /* Generate an unrepresentable length/address */ + do { + len = (1 << (pageshift + inc_page++)) + 1; + len = __builtin_align_up(len, pagesize); + } while (len == cheri_representable_length(len)); + + /* Create a memory mapping with reserved memory at the end */ + ptr1 = mmap(0, len, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr1)); + EXPECT_EQ(0, probe_mem_range(ptr1, len, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + /* Create a memory mapping with reserved memory at the front */ + ptr2 = mmap((void *)(uintcap_t)len, len, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr2)); + ASSERT_EQ(len, cheri_address_get(ptr2)); + EXPECT_EQ(0, probe_mem_range(ptr2, len, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + + retval = munmap(ptr1, len); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr2, len); + ASSERT_EQ(retval, 0); +} + int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -565,5 +602,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_mremap_bounds_check(); test_permissions(); test_brk_check(); + test_cheri_unrepresentability(); return 0; }
In title: I'd say s/addresses/bounds/
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
From: Amit Daniel Kachhap amit.kachhap@arm.com
Morello uses a compressed capability format which makes it difficult to represent bounds with arbitrary precision. As the corresponding address range of a given memory mapping may not be exactly representable as valid capability bounds, a test to verify that the PCuABI kernel is able to mmap/munmap those addresses has been added.
Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com Signed-off-by: Chaitanya S Prakash ChaitanyaS.Prakash@arm.com
tools/testing/selftests/arm64/morello/mmap.c | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index cb40bb88b1dd..50e1c8313ecd 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -552,6 +552,43 @@ TEST(test_brk_check) EXPECT_EQ(retval, -ENOSYS); } +/* test to verify the Cheri unrepresentable address/length */
"CHERI" (all capitals).
+TEST(test_cheri_unrepresentability) +{
- void *ptr1, *ptr2;
- int retval;
- int prot = PROT_READ | PROT_WRITE;
- int flags = MAP_PRIVATE | MAP_ANONYMOUS;
- size_t len;
- unsigned long inc_page = 0;
- /* Use pageshift 16 for 64K pages so as to use as mmap fixed address */
- unsigned long pageshift = 16;
- /* Generate an unrepresentable length/address */
- do {
len = (1 << (pageshift + inc_page++)) + 1;
I suppose pageshift could be directly incremented.
len = __builtin_align_up(len, pagesize);
That seems to be exactly equivalent to adding pagesize to len, instead of adding 1 above.
- } while (len == cheri_representable_length(len));
- /* Create a memory mapping with reserved memory at the end */
- ptr1 = mmap(0, len, prot, flags, -1, 0);
We should at least check the bounds of ptr1. Its length should be cheri_representable_length(len)), and its base aligned according to the alignment mask. Same idea for the second test.
Kevin
- ASSERT_FALSE(IS_ERR_VALUE(ptr1));
- EXPECT_EQ(0, probe_mem_range(ptr1, len, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
- /* Create a memory mapping with reserved memory at the front */
- ptr2 = mmap((void *)(uintcap_t)len, len, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr2));
- ASSERT_EQ(len, cheri_address_get(ptr2));
- EXPECT_EQ(0, probe_mem_range(ptr2, len, PROBE_MODE_TOUCH | PROBE_MODE_VERIFY));
- retval = munmap(ptr1, len);
- ASSERT_EQ(retval, 0);
- retval = munmap(ptr2, len);
- ASSERT_EQ(retval, 0);
+}
int main(__unused int argc, __unused char **argv, __unused char **envp, struct morello_auxv *auxv) { pagesize = get_pagesize(auxv); @@ -565,5 +602,6 @@ int main(__unused int argc, __unused char **argv, __unused char **envp, struct m test_mremap_bounds_check(); test_permissions(); test_brk_check();
- test_cheri_unrepresentability(); return 0;
}
This is V5 of the patch series, I have missed mentioning it in the subject line, I apologize for the confusion caused.
On 1/9/24 17:34, Chaitanya S Prakash wrote:
Syscalls operating on memory mappings manage their address space via owning capabilities. They must adhere to a certain set of rules[1] in order to ensure memory safety. Address space management syscalls are only allowed to manipulate mappings that are within the range of the owning capability and have appropriate permissions.
Tests to validate the capability's tag, bounds, range as well as permissions have been added. Finally, as certain flags and syscalls conflict with the reservation model or lack implementation, a check to verify appropriate handling of the same has also been added.
Review branch: https://git.morello-project.org/chaitanya_prakash/linux/-/tree/review/pureca...
This patch series has been tested on: https://git.morello-project.org/amitdaniel/linux/-/tree/review/purecap_mm_re...
[1]https://git.morello-project.org/morello/kernel/linux/-/wikis/Morello-pure-ca...
Changes in V5:
- Added representability testcase.
- Removed global struct reg_data and called get_pagesize() with auxv passed in main().
- Removed VERIFY_ERRNO macro and made use of extended EXPECT_EQ
- As helper functions have been removed, the inline attribute line is of no use and has been deleted.
- Used a common mapping to avoid creating and destroying mappings repeatedly.
- Removed positive testcases as they are not unique to PCuABI
- Corrected the error code to reflect -ENOMEM instead of -ERESERVATION when mremap() tries to move a mapping without MREMAP_MAYMOVE flag
- Reworded commit messages and restructured code.
Changes in V4: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
-Corrected subject of cover letter
Changes in V3: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
- Added get_pagesize() function and VERRIFY_ERRNO() macro
- Added LoadCap and StoreCap permissions testcase
- Added validity_tag_check testcases
- Added reservation tests
- Renamed variable "addr" to "ptr" to avoid confusion when manipulating both addresses and capabilities
- Cleaned up syscall_mmap and syscall_mmap2 testcases
- Restructured code into testcases that check tags, range, bounds and permissions
- Improved range_check testcases
- Improved commit messages
- Removed helper functions, tests directly written in testcase functions
- Removed signal handling and ddc register testcases
Changes in V2: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
- Added link to the review branch
- Removed unnecessary whitespace
Changes in V1: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/...
Amit Daniel Kachhap (1): kselftests/arm64: morello: mmap: Test unrepresentable addresses
Chaitanya S Prakash (10): kselftests/arm64: morello: Create wrapper functions for frequently invoked syscalls kselftests/arm64: morello: Add get_pagesize() function kselftests/arm64: morello: mmap: Clean up existing testcases kselftests/arm64: morello: mmap: Add MAP_GROWSDOWN testcase kselftests/arm64: morello: mmap: Add validity tag check testcases kselftests/arm64: morello: mmap: Add capability range testcases kselftests/arm64: morello: mmap: Add mmap() bounds check testcases kselftests/arm64: morello: mmap: Add mremap() bounds check testcases kselftests/arm64: morello: mmap: Add permission check testcases kselftests/arm64: morello: mmap: Add brk() testcase
.../selftests/arm64/morello/bootstrap.c | 13 - .../selftests/arm64/morello/freestanding.c | 15 + .../selftests/arm64/morello/freestanding.h | 74 ++- tools/testing/selftests/arm64/morello/mmap.c | 533 +++++++++++++++++- 4 files changed, 590 insertions(+), 45 deletions(-)
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
Syscalls operating on memory mappings manage their address space via owning capabilities. They must adhere to a certain set of rules[1] in order to ensure memory safety. Address space management syscalls are only allowed to manipulate mappings that are within the range of the owning capability and have appropriate permissions.
Tests to validate the capability's tag, bounds, range as well as permissions have been added. Finally, as certain flags and syscalls conflict with the reservation model or lack implementation, a check to verify appropriate handling of the same has also been added.
Well done, I think these tests are in a pretty good shape. Considering certain issues that I found while reviewing Amit's latest series, I would suggest adding the following tests (where you see fit):
- Ensuring that mmap(owning_cap, ..., MAP_FIXED) fails if the underlying reservation has been destroyed. The lifetime of reservations is generally undefined so this is hard to test if we only follow the spec, but it would make sense to have one test that relies on what we currently implement, i.e. the reservation is destroyed as soon as the last mapping is unmapped. So in practice something like ptr = mmap(...); munmap(ptr); mmap(ptr, ..., MAP_FIXED).
- Checking all capability permissions. I particular, special permissions like System are provided or not based on the caller's PCC, so it would be nice to check that removing System from PCC removes it from what mmap() returns too. Changing permissions in PCC is most easily done by adding a helper, creating a pointer to it, removing some permissions then calling that function pointer.
Kevin
Hi,
I will post the pcc permissions testcase as a standalone patch later.
On 1/17/24 22:36, Kevin Brodsky wrote:
On 09/01/2024 13:04, Chaitanya S Prakash wrote:
Syscalls operating on memory mappings manage their address space via owning capabilities. They must adhere to a certain set of rules[1] in order to ensure memory safety. Address space management syscalls are only allowed to manipulate mappings that are within the range of the owning capability and have appropriate permissions.
Tests to validate the capability's tag, bounds, range as well as permissions have been added. Finally, as certain flags and syscalls conflict with the reservation model or lack implementation, a check to verify appropriate handling of the same has also been added.
Well done, I think these tests are in a pretty good shape. Considering certain issues that I found while reviewing Amit's latest series, I would suggest adding the following tests (where you see fit):
- Ensuring that mmap(owning_cap, ..., MAP_FIXED) fails if the underlying
reservation has been destroyed. The lifetime of reservations is generally undefined so this is hard to test if we only follow the spec, but it would make sense to have one test that relies on what we currently implement, i.e. the reservation is destroyed as soon as the last mapping is unmapped. So in practice something like ptr = mmap(...); munmap(ptr); mmap(ptr, ..., MAP_FIXED).
- Checking all capability permissions. I particular, special permissions
like System are provided or not based on the caller's PCC, so it would be nice to check that removing System from PCC removes it from what mmap() returns too. Changing permissions in PCC is most easily done by adding a helper, creating a pointer to it, removing some permissions then calling that function pointer.
Kevin
linux-morello@op-lists.linaro.org
-
Chaitanya S Prakash -
Kevin Brodsky