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 the appropriate permissions. Tests to check 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.
The mincore() tests are expected to fail in this iteration as they are not fully supported. The next iterations will contain representability testcases.
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 V4: - 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/...
Chaitanya S Prakash (11): kselftests/arm64: morello: Create wrapper functions for frequently invoked syscalls kselftests/arm64: morello: Add get_pagesize() function kselftests/arm64: morello: Add VERIFY_ERRNO() macro 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 | 16 +- .../selftests/arm64/morello/freestanding.h | 74 ++- tools/testing/selftests/arm64/morello/mmap.c | 547 +++++++++++++++++- 4 files changed, 606 insertions(+), 44 deletions(-)
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 */
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 --- .../testing/selftests/arm64/morello/bootstrap.c | 13 ------------- .../selftests/arm64/morello/freestanding.c | 16 +++++++++++++++- .../selftests/arm64/morello/freestanding.h | 15 +++++++++++++++ tools/testing/selftests/arm64/morello/mmap.c | 12 +++++++++++- 4 files changed, 41 insertions(+), 15 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..a3d72482db7c 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.c +++ b/tools/testing/selftests/arm64/morello/freestanding.c @@ -6,7 +6,7 @@ #include <stdbool.h>
#include <linux/errno.h> - +#include <linux/auxvec.h> #include "freestanding.h"
/* @@ -93,6 +93,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..18b0cc6b3549 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
+static struct initial_data reg_data; + +static unsigned long pagesize;
static inline int probe_mem_range(void *addr, size_t size, int mode) { @@ -127,8 +130,15 @@ TEST(test_syscall_mmap2) syscall_mmap2(); }
-int main(void) +int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { + reg_data.argc = argc; + reg_data.argv = argv; + reg_data.envp = envp; + reg_data.auxv = auxv; + + pagesize = get_pagesize(reg_data.auxv); + test_syscall_mmap(); test_syscall_mmap2(); return 0;
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
[...]
diff --git a/tools/testing/selftests/arm64/morello/freestanding.c b/tools/testing/selftests/arm64/morello/freestanding.c index 45c0fa8b0914..a3d72482db7c 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.c +++ b/tools/testing/selftests/arm64/morello/freestanding.c @@ -6,7 +6,7 @@ #include <stdbool.h> #include <linux/errno.h>
Nit: that empty line was intentional - it's common to have one between global and local includes.
+#include <linux/auxvec.h> #include "freestanding.h" /* @@ -93,6 +93,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..18b0cc6b3549 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 +static struct initial_data reg_data;
It doesn't look like we actually need this global - get_pagesize() could be called with the auxv passed to main() directly.
Kevin
+static unsigned long pagesize; static inline int probe_mem_range(void *addr, size_t size, int mode) { @@ -127,8 +130,15 @@ TEST(test_syscall_mmap2) syscall_mmap2(); } -int main(void) +int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) {
- reg_data.argc = argc;
- reg_data.argv = argv;
- reg_data.envp = envp;
- reg_data.auxv = auxv;
- pagesize = get_pagesize(reg_data.auxv);
- test_syscall_mmap(); test_syscall_mmap2(); return 0;
VERIFY_ERRNO() macro is added to perform error handling. It is used to print the return value if the syscall fails with an error code other than what is expected.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 18b0cc6b3549..b04c754369b6 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -19,6 +19,13 @@ #define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02
+#define VERIFY_ERRNO(ret_val, expected_errno) \ + do { \ + if (ret_val != expected_errno) { \ + TH_LOG("Syscall failed with: %ld", (unsigned long)ret_val); \ + } \ + } while (0) + static struct initial_data reg_data;
static unsigned long pagesize;
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
VERIFY_ERRNO() macro is added to perform error handling. It is used to print the return value if the syscall fails with an error code other than what is expected.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
tools/testing/selftests/arm64/morello/mmap.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 18b0cc6b3549..b04c754369b6 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -19,6 +19,13 @@ #define PROBE_MODE_TOUCH 0x01 #define PROBE_MODE_VERIFY 0x02 +#define VERIFY_ERRNO(ret_val, expected_errno) \
- do { \
if (ret_val != expected_errno) { \TH_LOG("Syscall failed with: %ld", (unsigned long)ret_val); \} \- } while (0)
I'm not sure I see much value in this macro, compared to simply using EXPECT_EQ() or ASSERT_EQ() as appropriate, depending on whether we want to abort or not. They will print both the expected and actual value, as well as the current line; together these allow understanding what happened and where. For unexpected error messages, this is what matters most.
On closer inspection, it looks like our current implementation of __EXPECT() does not print the actual values, just the argument names. I'll make a note to fix that (making it closer to tools/testing/selftests/kselftest_harness.h).
Kevin
static struct initial_data reg_data; static unsigned long pagesize;
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 | 52 ++++++++------------ 1 file changed, 21 insertions(+), 31 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index b04c754369b6..e94f7d349705 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -30,9 +30,9 @@ static struct initial_data reg_data;
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) { @@ -51,32 +51,32 @@ static inline int probe_mem_range(void *addr, size_t size, int mode) * 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;
@@ -92,51 +92,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(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc;
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
[...]
@@ -51,32 +51,32 @@ static inline int probe_mem_range(void *addr, size_t size, int mode)
- in the virtual address space of the calling process
*/ static inline __attribute__((always_inline))
This line made sense when the helper was supposed to be inlined in the testcase function, but we can (and probably should) remove it now.
Kevin
-void syscall_mmap(void) +TEST(test_syscall_mmap) { [...]
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 e94f7d349705..831b8832bb74 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -127,6 +127,17 @@ 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); + VERIFY_ERRNO((unsigned long)ptr, (unsigned long)-EOPNOTSUPP); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -138,5 +149,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv)
test_syscall_mmap(); test_syscall_mmap2(); + test_map_growsdown(); return 0; }
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 | 111 +++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 831b8832bb74..8a4cc40e6095 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -138,6 +138,116 @@ TEST(test_map_growsdown) VERIFY_ERRNO((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); + VERIFY_ERRNO((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); + VERIFY_ERRNO(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); + VERIFY_ERRNO((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); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to madvise() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = madvise(cheri_tag_clear(ptr), MMAP_SIZE, MADV_WILLNEED); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to mincore() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mincore(cheri_tag_clear(ptr), MMAP_SIZE, vec); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to mlock() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mlock(cheri_tag_clear(ptr), MMAP_SIZE); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to mlock2() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mlock2(cheri_tag_clear(ptr), MMAP_SIZE, MLOCK_ONFAULT); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to munlock() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + 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); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munlock(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* passing invalid capability to msync() */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = msync(cheri_tag_clear(ptr), MMAP_SIZE, MS_SYNC); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -150,5 +260,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_syscall_mmap(); test_syscall_mmap2(); test_map_growsdown(); + test_validity_tag_check(); return 0; }
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
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 | 111 +++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 831b8832bb74..8a4cc40e6095 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -138,6 +138,116 @@ TEST(test_map_growsdown) VERIFY_ERRNO((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);- VERIFY_ERRNO((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);
- VERIFY_ERRNO(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);- VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-EINVAL);
From here on we could just call the remaining syscalls in a row, without creating and destroying a mapping every time, since every syscall is supposed to fail (and therefore have no effect). We could do something similar in patch 7 too.
Kevin
- 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);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to madvise() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = madvise(cheri_tag_clear(ptr), MMAP_SIZE, MADV_WILLNEED);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to mincore() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = mincore(cheri_tag_clear(ptr), MMAP_SIZE, vec);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to mlock() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = mlock(cheri_tag_clear(ptr), MMAP_SIZE);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to mlock2() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = mlock2(cheri_tag_clear(ptr), MMAP_SIZE, MLOCK_ONFAULT);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to munlock() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- 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);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munlock(ptr, MMAP_SIZE_REDUCED);
- ASSERT_EQ(retval, 0);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
- /* passing invalid capability to msync() */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = msync(cheri_tag_clear(ptr), MMAP_SIZE, MS_SYNC);
- VERIFY_ERRNO(retval, -EINVAL);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
+}
int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -150,5 +260,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_syscall_mmap(); test_syscall_mmap2(); test_map_growsdown();
- test_validity_tag_check(); return 0;
}
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 | 147 +++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 8a4cc40e6095..7b1a247719f4 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -248,6 +248,152 @@ 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_SHARED | 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(ptr)); + EXPECT_EQ(0, probe_mem_range(ptr, 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); + VERIFY_ERRNO((unsigned long)ret, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* negative munmap() range test */ + 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); + retval = munmap(reduced_bound_ptr, MMAP_SIZE); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* positive madvise() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = madvise(ptr, MMAP_SIZE, MADV_WILLNEED); + 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); + + /* negative madvise() range test */ + 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); + retval = madvise(reduced_bound_ptr, MMAP_SIZE, MADV_NORMAL); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* positive mincore() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mincore(ptr, MMAP_SIZE, vec); + 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); + + /* negative mincore() range test */ + 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); + retval = mincore(reduced_bound_ptr, MMAP_SIZE, vec); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* positive mlock() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = mlock(ptr, MMAP_SIZE); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + retval = munlock(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* negative mlock() range test */ + 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); + retval = mlock(reduced_bound_ptr, MMAP_SIZE); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* negative munlock() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + EXPECT_EQ(0, mlock2(ptr, MMAP_SIZE, MLOCK_ONFAULT)); + EXPECT_EQ(0, probe_mem_range(ptr, MMAP_SIZE, + PROBE_MODE_TOUCH | PROBE_MODE_VERIFY)); + reduced_bound_ptr = cheri_bounds_set(ptr, MMAP_SIZE_REDUCED); + retval = munlock(reduced_bound_ptr, MMAP_SIZE); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munlock(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); + + /* positive msync() range test */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + retval = msync(ptr, MMAP_SIZE, MS_SYNC); + 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); + + /* negative msync() range test */ + 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); + retval = msync(reduced_bound_ptr, MMAP_SIZE, MS_SYNC); + VERIFY_ERRNO(retval, -EINVAL); + + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -261,5 +407,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_syscall_mmap2(); test_map_growsdown(); test_validity_tag_check(); + test_range_check(); return 0; }
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
[...]
- /* positive madvise() range test */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- retval = madvise(ptr, MMAP_SIZE, MADV_WILLNEED);
- ASSERT_EQ(retval, 0);
The positive tests are essentially checking that the syscalls work in normal conditions. I think we could skip those, as for such standard conditions we can rely on the standard test suites (e.g. LTP). What we really want to test here is behaviour that is unique to PCuABI.
Kevin
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 the capability are found to overlap with the bounds of another mapping, the syscall fails with a -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 | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 7b1a247719f4..a2bf0c09dac1 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -394,6 +394,39 @@ TEST(test_range_check) ASSERT_EQ(retval, 0); }
+/* test to verify mmap() behaviour when capability bounds are modified */ +TEST(test_mmap_bounds_check) +{ + void *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); + + /* overlapping mappings produce a reservation error */ + ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0); + ASSERT_FALSE(IS_ERR_VALUE(ptr)); + new_ptr = mmap(ptr + MMAP_SIZE_REDUCED, MMAP_SIZE_REDUCED, prot, + flags | MAP_FIXED, -1, 0); + VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-ERESERVATION); + retval = munmap(ptr, MMAP_SIZE); + ASSERT_EQ(retval, 0); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -408,5 +441,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_map_growsdown(); test_validity_tag_check(); test_range_check(); + test_mmap_bounds_check(); return 0; }
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
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 the capability are found to overlap with the bounds of another mapping, the syscall fails with a -ERESERVATION
It looks like there is some confusion between mapping and reservation here, -ERESERVATION is not related to mappings. Overall I'm not sure I understand the second test, which this sentence seems to describe. AFAICT it is essentially the same as the first one in patch 7, and shouldn't fail. I think what you are trying to test here is the case where ptr is *null-derived*, in which case an overlap with an existing reservation will indeed cause -ERESERVATION.
Kevin
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 | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 7b1a247719f4..a2bf0c09dac1 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -394,6 +394,39 @@ TEST(test_range_check) ASSERT_EQ(retval, 0); } +/* test to verify mmap() behaviour when capability bounds are modified */ +TEST(test_mmap_bounds_check) +{
- void *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);
- /* overlapping mappings produce a reservation error */
- ptr = mmap(NULL, MMAP_SIZE, prot, flags, -1, 0);
- ASSERT_FALSE(IS_ERR_VALUE(ptr));
- new_ptr = mmap(ptr + MMAP_SIZE_REDUCED, MMAP_SIZE_REDUCED, prot,
flags | MAP_FIXED, -1, 0);- VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-ERESERVATION);
- retval = munmap(ptr, MMAP_SIZE);
- ASSERT_EQ(retval, 0);
+}
int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -408,5 +441,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_map_growsdown(); test_validity_tag_check(); test_range_check();
- test_mmap_bounds_check(); return 0;
}
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 -ERESERVATION error. 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 | 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 a2bf0c09dac1..645d6b977bdf 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -427,6 +427,44 @@ 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 *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, 0); + ASSERT_FALSE(IS_ERR_VALUE(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 a reservation 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); + VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-ERESERVATION); + + 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); + VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-EINVAL); + + retval = munmap(ptr, MMAP_SIZE_REDUCED); + ASSERT_EQ(retval, 0); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -442,5 +480,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_validity_tag_check(); test_range_check(); test_mmap_bounds_check(); + test_mremap_bounds_check(); return 0; }
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
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 -ERESERVATION error. 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 | 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 a2bf0c09dac1..645d6b977bdf 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -427,6 +427,44 @@ 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 *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, 0);
Nit: using NULL instead of 0 is more readable for pointers.
- ASSERT_FALSE(IS_ERR_VALUE(new_ptr));
What we should also check here is that new_ptr != ptr (i.e. the mapping was moved). Aside from that the behaviour is the same regardless of the ABI.
- 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 a reservation 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);
- VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-ERESERVATION);
While mremap() indeed fails because of the insufficient size of the reservation, the expected error is not -ERESERVATION. Indeed, as per the spec:
The semantics of MREMAP_MAYMOVE is modified so that it is considered
that a mapping must be moved if the range (old_address.address, new_size) does not fit in the reservation to which the old mapping belongs, in addition to existing conditions (i.e. not clashing with existing mappings).
In other words, in this case, the mapping is required to be moved, but since MREMAP_MAYMOVE is not passed, the call fails. Regardless of why exactly the mapping should be moved, the expected error is -ENOMEM (as per mremap(2)).
Kevin
- 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);
- VERIFY_ERRNO((unsigned long)new_ptr, (unsigned long)-EINVAL);
- retval = munmap(ptr, MMAP_SIZE_REDUCED);
- ASSERT_EQ(retval, 0);
+}
int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -442,5 +480,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_validity_tag_check(); test_range_check(); test_mmap_bounds_check();
- test_mremap_bounds_check(); return 0;
}
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 a -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 | 130 +++++++++++++++++++ 1 file changed, 130 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 645d6b977bdf..5c6c8e8efd75 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 @@ -465,6 +466,134 @@ TEST(test_mremap_bounds_check) ASSERT_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; + + addr = (void *)(uintcap_t)pagesize; + + /* 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); + VERIFY_ERRNO((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 */ + 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); + VERIFY_ERRNO((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(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -481,5 +610,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_range_check(); test_mmap_bounds_check(); test_mremap_bounds_check(); + test_permissions(); return 0; }
On 03/10/2023 07:41, Chaitanya S Prakash wrote:
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 a -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 | 130 +++++++++++++++++++ 1 file changed, 130 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 645d6b977bdf..5c6c8e8efd75 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 @@ -465,6 +466,134 @@ TEST(test_mremap_bounds_check) ASSERT_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;
- addr = (void *)(uintcap_t)pagesize;
It would be more readable to move it next to where it is used (since it used just once). Also uintptr_t is preferable to uintcap_t (the former suggests a pointer represented as integer in general, while the latter suggests something that is actually specific to capabilities).
- /* 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);
- VERIFY_ERRNO((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 */
- 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);
prot | PROT_MAX(prot) is equivalent to just prot. We don't need to specify MAX_PROT() in every testcase.
Kevin
[...]
Hi,
On 10/3/23 11:11, Chaitanya S Prakash wrote:
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 a -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 | 130 +++++++++++++++++++ 1 file changed, 130 insertions(+)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 645d6b977bdf..5c6c8e8efd75 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 @@ -465,6 +466,134 @@ TEST(test_mremap_bounds_check) ASSERT_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;
- addr = (void *)(uintcap_t)pagesize;
- /* 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);
- VERIFY_ERRNO((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 */
- 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);
Here, addr has to be >= to mmap_min_addr which in kernel is 8K. So you may set addr to 8K to get the same mmap fixed address.
Thanks, Amit
- 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);- VERIFY_ERRNO((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(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc;
@@ -481,5 +610,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_range_check(); test_mmap_bounds_check(); test_mremap_bounds_check();
- test_permissions(); return 0; }
On 21/11/2023 09:54, Amit Daniel Kachhap wrote:
On 10/3/23 11:11, Chaitanya S Prakash wrote:
[...]
+ /* repeat positive max_prot test with fixed address */ + 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);
Here, addr has to be >= to mmap_min_addr which in kernel is 8K. So you may set addr to 8K to get the same mmap fixed address.
That's a very good point, I didn't realise that the minimum address is configurable. Looking at security/min_addr.c, it seems that there are multiple values to consider. To err on the side of caution, I would use the highest value, i.e. the default value of CONFIG_LSM_MMAP_MIN_ADDR, 65536 (64K). In fact the user is free to choose an even higher value, but I think that's a reasonable compromise.
Kevin
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 5c6c8e8efd75..8e0d797a74a2 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -594,6 +594,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); + VERIFY_ERRNO(retval, -ENOSYS); +} + int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) { reg_data.argc = argc; @@ -611,5 +620,6 @@ int main(int argc, char **argv, char **envp, struct morello_auxv *auxv) test_mmap_bounds_check(); test_mremap_bounds_check(); test_permissions(); + test_brk_check(); return 0; }
linux-morello@op-lists.linaro.org
-
Amit Daniel Kachhap -
Chaitanya S Prakash -
Kevin Brodsky