Add tests for capability enforcement via the standard copy routines (copy_{to, from}_user, {get, put}_user), futex operations and the explicit uaccess checks used in iov_iter via readv and writev.
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com ---
Hi everyone,
This is v4 of the uaccess kselftest patch and introduces the following: - Simplification of the futex test to only use one futex - Move FUTEX_WAKE_OP description to futex test - Various minor corrections
V3 introduced the following changes: - Corrections to function prototypes to remove void * in favour of the actual struct pointer types and other minor corrections. - Correction to the futex test, which now tests for a correctly enforced uaccess routine when used in an atomic operation. - Reintroduction of the out-of-bounds checking in tests.
V2 introduced the following changes: - More concise formatting changes - Corrections to various mistakes in tests - Addition of a strlen_user() test - Fixed futex test
The review branch can be found at: https://git.morello-project.org/arkamnite/linux/-/commits/morello/uaccess_v4
The related issue can also be found at: https://git.morello-project.org/morello/kernel/linux/-/issues/58
Many thanks,
Akram
--- .../testing/selftests/arm64/morello/Makefile | 2 +- .../testing/selftests/arm64/morello/uaccess.c | 262 ++++++++++++++++++ 2 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/arm64/morello/uaccess.c
diff --git a/tools/testing/selftests/arm64/morello/Makefile b/tools/testing/selftests/arm64/morello/Makefile index ecfa6fc5e989..7b794d3e436c 100644 --- a/tools/testing/selftests/arm64/morello/Makefile +++ b/tools/testing/selftests/arm64/morello/Makefile @@ -17,7 +17,7 @@ CFLAGS += $(CLANG_FLAGS) $(CFLAGS_PURECAP) $(CFLAGS_COMMON) LDFLAGS += $(CLANG_LDFLAGS) $(CLANG_FLAGS) -nostdlib -static
SRCS := $(wildcard *.c) $(wildcard *.S) -PROGS := bootstrap clone exit mmap read_write sched signal +PROGS := bootstrap clone exit mmap read_write sched signal uaccess DEPS := $(wildcard *.h)
# these are the final executables diff --git a/tools/testing/selftests/arm64/morello/uaccess.c b/tools/testing/selftests/arm64/morello/uaccess.c new file mode 100644 index 000000000000..ee2847472bc2 --- /dev/null +++ b/tools/testing/selftests/arm64/morello/uaccess.c @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2023 Arm Limited + +#include <stdint.h> +#include <stddef.h> +#include <stdbool.h> + +#include <asm/fcntl.h> +#include <asm-generic/errno-base.h> +#include <cheriintrin.h> +#include <linux/errno.h> +#include <linux/fs.h> +#include <linux/futex.h> +#include <linux/mman.h> +#include <linux/signal.h> +#include <linux/sysinfo.h> +#include <linux/socket.h> +#include <linux/time.h> +#include <linux/uio.h> + +#include "freestanding.h" + +#define MSG_LEN 16 + +static const char file[] = "/my_file.txt"; + +static inline int fsopen(const char *string) +{ + return syscall(__NR_fsopen, string); +} + + +static inline int futex_wake_op(uint32_t *uaddr, uint32_t *uaddr2, uint32_t val3) +{ + return syscall(__NR_futex, uaddr, FUTEX_WAKE_OP, 1, 1, + uaddr2, val3); +} + +typedef unsigned short sa_family_t; + +struct sockaddr { + sa_family_t sa_family; + char sa_data[14]; +}; + +static int getsockname(int socket, struct sockaddr *sockaddr, int *socklen) +{ + return syscall(__NR_getsockname, socket, sockaddr, socklen); +} + +static int open_file(void) +{ + return syscall(__NR_openat, 0, file, O_RDWR | O_CREAT, 0666); +} + +static inline int readv(int fd, struct iovec *iov, int iovcnt) +{ + return syscall(__NR_readv, fd, iov, iovcnt); +} + +static inline int writev(int fd, struct iovec *iov, int iovcnt) +{ + return syscall(__NR_writev, fd, iov, iovcnt); +} + +static inline int sigaction(int sig, struct sigaction *old, struct sigaction *new) +{ + return syscall(__NR_rt_sigaction, sig, old, new, sizeof(sigset_t)); +} + +static inline int sysinfo(struct sysinfo *ptr) +{ + return syscall(__NR_sysinfo, ptr); +} + +TEST(test_copy_to_user) +{ + struct sysinfo my_sysinfo; + + ASSERT_EQ(sysinfo(&my_sysinfo), 0); + + ASSERT_EQ(sysinfo(&my_sysinfo + sizeof(struct sysinfo)), -EFAULT); + + ASSERT_EQ(sysinfo(cheri_tag_clear(&my_sysinfo)), -EFAULT); + + ASSERT_EQ(sysinfo(cheri_perms_and(&my_sysinfo, 0)), -EFAULT); +} + +/* + * sigaction() tests both copy_to_user and copy_from_user by copying + * into and from a pointer to struct sigaction. The resulting action + * in this case is a no-op, but this does not affect the validity of + * the test. sigaction attempts to copy the new action from user space + * and the old action to user space, if the arguments is valid in either + * case. + */ +TEST(test_copy_user) +{ + struct sigaction act; + + ASSERT_EQ(sigaction(SIGSEGV, NULL, &act), 0); + ASSERT_EQ(sigaction(SIGSEGV, &act, NULL), 0); + + ASSERT_EQ(sigaction(SIGSEGV, NULL, &act + sizeof(struct sigaction)), -EFAULT); + ASSERT_EQ(sigaction(SIGSEGV, &act + sizeof(struct sigaction), NULL), -EFAULT); + + ASSERT_EQ(sigaction(SIGSEGV, NULL, cheri_tag_clear(&act)), -EFAULT); + ASSERT_EQ(sigaction(SIGSEGV, cheri_tag_clear(&act), NULL), -EFAULT); + + ASSERT_EQ(sigaction(SIGSEGV, NULL, cheri_perms_and(&act, 0)), -EFAULT); + ASSERT_EQ(sigaction(SIGSEGV, cheri_perms_and(&act, 0), NULL), -EFAULT); +} + +/* + * getsockname(2) uses both get_user and put_user to copy the addrlen argument + * from and to user space respectively. getsockname(2) calls move_addr_to_user + * to copy an address to user space. move_addr_to_user also uses copy_to_user; + * if copy_to_user is successful, but either of get_user or put_user fail then + * getsockname(2) will return -EFAULT appropriately. + */ +TEST(test_get_put_user) +{ + struct sockaddr sa; + int sa_len = sizeof(sa); + int my_socket; + + // socket(AF_INET, SOCK_STREAM, 0) + my_socket = syscall(__NR_socket, 2, 1, 0); + + // The socket must be successfully opened before proceeding. + ASSERT_GE(my_socket, 0); + + ASSERT_EQ(getsockname(my_socket, &sa, &sa_len), 0); + ASSERT_EQ(getsockname(my_socket, &sa, &sa_len + sizeof(struct sockaddr)), -EFAULT); + ASSERT_EQ(getsockname(my_socket, &sa, cheri_tag_clear(&sa_len)), -EFAULT); + ASSERT_EQ(getsockname(my_socket, &sa, cheri_perms_and(&sa_len, 0)), -EFAULT); + close(my_socket); +} + +/* + * The FUTEX_WAKE_OP operation in the futex(2) syscall is used to handle more + * than one futex at the same time. This is an atomic operation that attempts + * to read, modify and write a value at a second uaddr (uaddr2) and also wake + * up waiters on the futex words at uaddr1 and uaddr2. uaddr1 and uaddr2 are + * provided via capabilities, and so this futex tests the case of an atomic + * uaccess operation. + * + * When using FUTEX_WAKE_OP, val3 is an encoding of both the operation as + * well as the comparison to be performed on the futex word at uaddr2. If val3 + * is set to 0, this represents an operation of FUTEX_OP_SET and a comparison + * FUTEX_OP_CMP_EQ, where both oparg and comparg are 0. + */ +TEST(test_futex) +{ + static struct futex_waitv waitv; + uint32_t futex = 0; + + waitv.uaddr = (uintptr_t)&futex; + waitv.flags = FUTEX_32 | FUTEX_PRIVATE_FLAG; + waitv.val = futex; + waitv.__reserved = 0; + + ASSERT_GE(futex_wake_op(&futex, &futex, 0), 0); + ASSERT_EQ(futex_wake_op(&futex, &futex + sizeof(uint32_t), 0), -EFAULT); + ASSERT_EQ(futex_wake_op(&futex, cheri_tag_clear(&futex), 0), -EFAULT); + ASSERT_EQ(futex_wake_op(&futex, cheri_perms_and(&futex, 0), 0), + -EFAULT); +} + +/* + * Test explicit accesses used in iov_iter via readv and writev. Both + * syscalls use explicit checking on the iov_base field of struct iovec, + * so the metadata of the capability provided for iov_base is modified as + * per the needs of each individual test. + */ +TEST(test_explicit_iov_iter) +{ + char buf0[2]; + char buf1[4]; + char buf2[6]; + const char *write_buf0 = "Hello I am the first char buffer!\n"; + const char *write_buf1 = "Hello, I am the second char buffer.\n"; + const char *write_buf2 = "Hello, I am the third and final char buffer.\n"; + struct iovec iov[3]; + int iovcnt; + int fd; + + fd = open_file(); + ASSERT_NE(fd, -1); + + iov[0].iov_base = buf0; + iov[0].iov_len = sizeof(buf0); + iov[1].iov_base = buf1; + iov[1].iov_len = sizeof(buf1); + iov[2].iov_base = buf2; + iov[2].iov_len = sizeof(buf2); + + iovcnt = sizeof(iov) / sizeof(struct iovec); + + ASSERT_GE(readv(fd, iov, iovcnt), 0); + + iov[0].iov_base = buf0 + sizeof(iov) * 100; + ASSERT_EQ(readv(fd, iov, iovcnt), -EFAULT); + + iov[0].iov_base = cheri_tag_clear(buf0); + ASSERT_EQ(readv(fd, iov, iovcnt), -EFAULT); + + iov[0].iov_base = cheri_perms_and(buf0, 0); + ASSERT_EQ(readv(fd, iov, iovcnt), -EFAULT); + + iov[0].iov_base = (void *)write_buf0; + iov[0].iov_len = strlen(write_buf0); + iov[1].iov_base = (void *)write_buf1; + iov[1].iov_len = strlen(write_buf1); + iov[2].iov_base = (void *)write_buf2; + iov[2].iov_len = strlen(write_buf2); + + ASSERT_GE(writev(fd, iov, iovcnt), 0); + + iov[0].iov_base = (void *)write_buf0 + sizeof(iov) * 100; + ASSERT_EQ(writev(fd, iov, iovcnt), -EFAULT); + + iov[0].iov_base = (void *)cheri_tag_clear(write_buf0); + ASSERT_EQ(writev(fd, iov, iovcnt), -EFAULT); + + iov[0].iov_base = (void *)cheri_perms_and(write_buf0, 0); + ASSERT_EQ(writev(fd, iov, iovcnt), -EFAULT); + + close(fd); +} + +/* + * strlen_user() explicitly inspects an input capability, the behaviour + * of which must also be verified within these tests. The fsopen() syscall + * makes use of strlen_user() by duplicating a string (representing the name + * of a filesystem) with the strndup_user() function. strlen_user() can + * therefore be tested with a call to fsopen(). + */ +TEST(test_strlen_user) +{ + const char *fsname = "my_nonexistent_filesystem"; + + // strndup_user() will still be called, so fsopen() fails after this. + ASSERT_EQ(fsopen(fsname), -ENODEV); + + ASSERT_EQ(fsopen(fsname + strlen(fsname) * 10), -EFAULT); + + ASSERT_EQ(fsopen(cheri_tag_clear(fsname)), -EFAULT); + + ASSERT_EQ(fsopen(cheri_perms_and(fsname, 0)), -EFAULT); +} + +int main(void) +{ + test_copy_to_user(); + test_copy_user(); + test_get_put_user(); + test_futex(); + test_explicit_iov_iter(); + test_strlen_user(); + return 0; +}
On 18/01/2024 18:31, Akram Ahmad wrote:
[...]
+TEST(test_copy_to_user) +{
- struct sysinfo my_sysinfo;
- ASSERT_EQ(sysinfo(&my_sysinfo), 0);
- ASSERT_EQ(sysinfo(&my_sysinfo + sizeof(struct sysinfo)), -EFAULT);
We are still using a byte offset of sizeof(struct sysinfo) * sizeof(struct sysinfo). Arithmetic operations on T * work at a granularity of sizeof(T), so incrementing a T * pointer by 1 increases its address by sizeof(T), and that should be enough here.
- ASSERT_EQ(sysinfo(cheri_tag_clear(&my_sysinfo)), -EFAULT);
- ASSERT_EQ(sysinfo(cheri_perms_and(&my_sysinfo, 0)), -EFAULT);
+}
[...]
+TEST(test_futex) +{
- static struct futex_waitv waitv;
This is not actually used any more.
- uint32_t futex = 0;
- waitv.uaddr = (uintptr_t)&futex;
- waitv.flags = FUTEX_32 | FUTEX_PRIVATE_FLAG;
- waitv.val = futex;
- waitv.__reserved = 0;
- ASSERT_GE(futex_wake_op(&futex, &futex, 0), 0);
- ASSERT_EQ(futex_wake_op(&futex, &futex + sizeof(uint32_t), 0), -EFAULT);
- ASSERT_EQ(futex_wake_op(&futex, cheri_tag_clear(&futex), 0), -EFAULT);
- ASSERT_EQ(futex_wake_op(&futex, cheri_perms_and(&futex, 0), 0),
-EFAULT);
+}
+/*
- Test explicit accesses used in iov_iter via readv and writev. Both
- syscalls use explicit checking on the iov_base field of struct iovec,
- so the metadata of the capability provided for iov_base is modified as
- per the needs of each individual test.
- */
+TEST(test_explicit_iov_iter) +{
- char buf0[2];
- char buf1[4];
- char buf2[6];
- const char *write_buf0 = "Hello I am the first char buffer!\n";
- const char *write_buf1 = "Hello, I am the second char buffer.\n";
- const char *write_buf2 = "Hello, I am the third and final char buffer.\n";
- struct iovec iov[3];
- int iovcnt;
- int fd;
- fd = open_file();
- ASSERT_NE(fd, -1);
- iov[0].iov_base = buf0;
- iov[0].iov_len = sizeof(buf0);
- iov[1].iov_base = buf1;
- iov[1].iov_len = sizeof(buf1);
- iov[2].iov_base = buf2;
- iov[2].iov_len = sizeof(buf2);
- iovcnt = sizeof(iov) / sizeof(struct iovec);
- ASSERT_GE(readv(fd, iov, iovcnt), 0);
- iov[0].iov_base = buf0 + sizeof(iov) * 100;
In this case sizeof(iov) has nothing to do with the address of iov_base (pointer to a string). Incrementing by 100 sounds reasonable.
Kevin
[...]
linux-morello@op-lists.linaro.org