On Mon, Dec 12, 2022 at 05:48:41PM +0100, Kevin Brodsky wrote:
On 08/12/2022 11:26, Beata Michalska wrote:
Add a bunch of test-cases for clone3 syscall, focusing mainly on handling struct clone_args versioning and sizing, alongside the obvious req for respecting capabilities.
Signed-off-by: Beata Michalska beata.michalska@arm.com
.../testing/selftests/arm64/morello/Makefile | 1 + tools/testing/selftests/arm64/morello/clone.c | 232 +++++++++++++++++- 2 files changed, 232 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/morello/Makefile b/tools/testing/selftests/arm64/morello/Makefile index 7dbf5b140695..c79e8973dbbf 100644 --- a/tools/testing/selftests/arm64/morello/Makefile +++ b/tools/testing/selftests/arm64/morello/Makefile @@ -38,3 +38,4 @@ $(OUTPUT)/%: $(OUTPUT)/%.o $(OUTPUT)/freestanding_start.o $(OUTPUT)/freestanding $(CC) $^ -o $@ $(LDFLAGS) $(OUTPUT)/signal: $(OUTPUT)/signal_common.o +$(OUTPUT)/clone: $(OUTPUT)/signal_common.o diff --git a/tools/testing/selftests/arm64/morello/clone.c b/tools/testing/selftests/arm64/morello/clone.c index 8217840ba504..bc38f2673e9f 100644 --- a/tools/testing/selftests/arm64/morello/clone.c +++ b/tools/testing/selftests/arm64/morello/clone.c @@ -1,15 +1,20 @@ // SPDX-License-Identifier: GPL-2.0 // Copyright (C) 2021 Arm Limited +#include "freestanding.h" #include <linux/mman.h> #include <linux/resource.h> #include <linux/wait.h> #include <linux/signal.h> #include <linux/sched.h> #include <linux/errno.h> +#include <linux/signal.h> +#include <linux/types.h> +#include <limits.h> #include <cheriintrin.h> -#include "freestanding.h" +#include "signal_common.h" #define STACK_SIZE 1024*1024 +#define TLS_SIZE 4096 #define in_restricted() \ (!(cheri_perms_get(cheri_pcc_get()) & ARM_CAP_PERMISSION_EXECUTIVE)) @@ -17,6 +22,10 @@ #define MAGIC_NR 0x3562 /* whatever really ..... */ +#ifndef MAX_PID_NS_LEVEL +#define MAX_PID_NS_LEVEL 32 +#endif
/* Cloned thread result */ #define CTR_SUCCESS 1 #define CTR_FAILED -1 @@ -256,6 +265,226 @@ TEST(test_wait_rusage) RUN_WITH_FIXTURE(data, CLONE_TH_RUSAGE, wait_single); } +#define GET_ERRNO(err) (((err) + MAX_ERRNO) ^ MAX_ERRNO)
+/*
- With clone3 there is still plenty of room within __u64
- for new flags so it should be safe to claim top bit here...
- */
+#define CUSTOM_CLONE_STACK_INV (1ULL << ((sizeof(__u64) * 8) -1))
I'm not a big fan of putting this directly in struct clone_args, it doesn't feel that the dance is justified. Having some .test_flags member in struct clone3_fixture should work fine?
Should / would / will.
+static struct clone3_fixture {
- size_t args_size;
- struct clone_args args;
- int e_result;
+} clone3_data[] = {
- /* BEGIN_SECTION: expected failure */
- /* size of clone_args smaller than CLONE_ARGS_SIZE_VER0 */
- {
.args_size = offsetof(struct clone_args, tls),.e_result = -EINVAL- }, /* @{0} */
- /* invalid set_tid array size */
- {
.args_size = sizeof(struct clone_args),.args.set_tid_size = MAX_PID_NS_LEVEL + 1,.e_result = -EINVAL- }, /* @{1} */
- /* Invalid combination of set_tid & set_tid_size */
- {
.args_size = sizeof(struct clone_args),.args.set_tid_size = 1,.e_result = -EINVAL- }, /* @{2} */
- /* Invalid exit_signal */
- {
.args_size = sizeof(struct clone_args),.args.exit_signal = _NSIG + 1,.e_result = -EINVAL- }, /* @{3} */
- /* Invalid cgroup number */
- {
.args_size = sizeof(struct clone_args),.args.flags = CLONE_INTO_CGROUP,.args.cgroup = (__u64)INT_MAX + 1,.e_result = -EINVAL- }, /* @{4} */
- /* Invalid size for clone_args with cgroup */
- {
.args_size = offsetof(struct clone_args, cgroup),.args.flags = CLONE_INTO_CGROUP,.args.cgroup = 1,.e_result = -EINVAL- }, /* @{5} */
- /* Invalid stack & stack_size combination */
- /* Use top bits of clone flags to mark those */
- {
.args_size = sizeof(struct clone_args),.args.flags = CUSTOM_CLONE_STACK_INV,.args.stack_size = STACK_SIZE,.e_result = -EINVAL- }, /* @{6} */
- {
.args_size = sizeof(struct clone_args),.args.flags = CUSTOM_CLONE_STACK_INV,.e_result = -EINVAL- }, /* @{7} */
- /* Invalid set_tid entry */
- {
.args_size = sizeof(struct clone_args),.args.set_tid = (uintptr_t)&(pid_t){1},.args.set_tid_size = 1,.e_result = -EEXIST- }, /* @{8} */
- /* END_SECTION: expected failure */
- {
.args_size = sizeof(struct clone_args),.args.flags = CLONE_PIDFD | CLONE_CHILD_SETTID |CLONE_CHILD_CLEARTID,.e_result = 0- }, /* @{9} */
- {
.args_size = sizeof(struct clone_args),.args.flags = CLONE_PARENT_SETTID | CLONE_CHILD_SETTID,.e_result = 0- }, /* @{10} */
- {
.args_size = sizeof(struct clone_args),.args.flags = CLONE_SETTLS,.e_result = 0- }, /* @{11} */
+};
+static __attribute__((noinline)) void run_child(struct clone_args *args) +{
- static __thread int tls_data;
- if (args->flags & CLONE_CHILD_SETTID) {
pid_t current_pid = getpid();ASSERT_FALSE(current_pid != *(pid_t *)args->child_tid);- }
- if (args->flags & CLONE_SETTLS && args->tls) {
ptraddr_t base_addr = cheri_address_get(args->tls);ptraddr_t ref_addr = cheri_address_get(&tls_data);size_t length = cheri_length_get(args->tls);ASSERT_TRUE(cheri_tag_get(args->tls));ASSERT_TRUE(ref_addr >= base_addr && ref_addr < base_addr + length);- }
- if (args->flags & CLONE_PIDFD) {
sigset_t set;ASSERT_EQ(sigemptyset(&set), 0);ASSERT_EQ(sigaddset(&set, SIGUSR1), 0);ASSERT_EQ(sigprocmask(SIG_BLOCK, &set, NULL), 0);/* Suspend utill parent kicks things back in */ASSERT_EQ(syscall(__NR_kill, getpid(), SIGSTOP), 0);/* Wait for a signal */ASSERT_EQ(rt_sigtimedwait(&set, NULL, 0, sizeof(set)),SIGUSR1);- }
- syscall(__NR_exit, 0);
+}
+static inline __attribute__((always_inline)) +void run_clone3(struct clone3_fixture *data) +{
- struct clone_args *args = &(data->args);
- int pidfd, parent_tid = 0, child_tid = 0;
- siginfo_t wait_si;
- int result;
- pid_t pid;
- args->pidfd = (uintcap_t)&pidfd;
- args->parent_tid = (uintcap_t)&parent_tid;
- args->child_tid = (uintcap_t)&child_tid;
- if (!args->exit_signal)
args->exit_signal = SIGCHLD;- if (!args->stack_size) {
args->stack = (uintcap_t) mmap_verified(NULL, STACK_SIZE,PROT_READ | PROT_WRITE,MAP_ANONYMOUS | MAP_PRIVATE,-1, 0, STACK_REQ_PERMS);args->stack_size =args->flags & CUSTOM_CLONE_STACK_INV ? 0 : STACK_SIZE;- }
- args->flags &= ~CUSTOM_CLONE_STACK_INV;
- if (data->e_result) {
result = syscall(__NR_clone3, args, data->args_size);ASSERT_EQ(result, data->e_result) {TH_LOG("Expected: %d while %d was received",GET_ERRNO(data->e_result), GET_ERRNO(result));}munmap((void *)args->stack, STACK_SIZE);Maybe check that args->stack is not null? Not essential but that's saving an unnecessary syscall.
Will do.
return;- }
- args->flags |= CLONE_VM;
- if (args->flags & CLONE_SETTLS) {
void *addr = mmap_verified(NULL, TLS_SIZE, PROT_READ | PROT_WRITE,MAP_ANONYMOUS | MAP_PRIVATE, -1, 0,CAP_LOAD_PERMS | CAP_STORE_PERMS);args->tls = (uintcap_t)cheri_bounds_set(addr, TLS_SIZE);- }
- pid = syscall(__NR_clone3, args, data->args_size);
- ASSERT_GE(pid, 0);
- if (!pid)
run_child(args);- if (args->flags & CLONE_PIDFD) {
/* Make sure the child here gets a chance to block the signal */result = waitid(P_PID, pid, &wait_si, WUNTRACED | WCONTINUED, NULL);The SIGSTOP / SIGCONT way is a smart way to approach this, I like it! Just one question regarding the flags, do we really need either?
we need smth ...
WUNTRACED means that a child stopping while being traced is ignored, and WCONTINUED means returning if the child receives SIGCONT. SIGSTOP causes waitid() to return by default.
switched to WSTOPPED instead.
Thanks for the comments. --- BR B.
Kevin
ASSERT_EQ(result, 0);result = syscall(__NR_kill, pid, SIGCONT);ASSERT_EQ(result, 0);/* Signal handling is not the test target here: valid pidfd is */result = syscall(__NR_pidfd_send_signal, pidfd, SIGUSR1, NULL, 0);ASSERT_EQ(result, 0);- }
- result = waitid(P_PID, pid, &wait_si, WEXITED, NULL);
- ASSERT_EQ(result, 0);
- /* child_tid set once the thread gets scheduled */
- if (args->flags & CLONE_PARENT_SETTID && args->flags & CLONE_CHILD_SETTID
&& !(args->flags & CLONE_CHILD_CLEARTID)) {ASSERT_EQ(parent_tid, child_tid);ASSERT_NE(parent_tid, 0);- }
- if (args->flags & CLONE_CHILD_CLEARTID)
ASSERT_EQ(child_tid, 0);- munmap((void *)args->stack, STACK_SIZE);
- if (args->flags & CLONE_SETTLS)
munmap((void *)args->tls, TLS_SIZE);+}
+TEST(test_clone3) +{
- int ncount = sizeof(clone3_data)/sizeof(clone3_data[0]);
- for (int i = 0; i < ncount; ++i) {
TH_LOG("Validating clone3 @{%d}", i);run_clone3(&clone3_data[i]);- }
+}
void run_restricted(uintcap_t entry_point) { void *new_stack = allocate_mem_raw(STACK_SIZE); @@ -290,5 +519,6 @@ int main(void) run_restricted((uintcap_t)test_clone_tls_restricted); test_wait_raw(); test_wait_rusage();
- test_clone3(); return 0;
}