The vector regset uses the maximum possible vlenb 8192 to allocate a 2^18 bytes buffer to copy the vector register. But most platforms don’t support the largest vlenb.
The regset has 2 users, ptrace syscall and coredump. When handling the PTRACE_GETREGSET requests from ptrace syscall, Linux will prepare a kernel buffer which size is min(user buffer size, limit). A malicious user process might overwhelm a memory-constrainted system when the buffer limit is very large. The coredump uses regset_get_alloc() to get the context of vector register. But this API allocates buffer before checking whether the target process uses vector extension, this wastes time to prepare a large memory buffer.
The buffer limit can be determined after getting platform vlenb in the early boot stage, this can let the regset buffer match real hardware limits. Also add .active callbacks to let the coredump skip vector part when target process doesn't use it.
After this patchset, userspace process needs 2 ptrace syscalls to retrieve the vector regset with PTRACE_GETREGSET. The first ptrace call only reads the header to get the vlenb information. Then prepare a suitable buffer to get the register context. The new vector ptrace kselftest demonstrates it.
--- v2: - fix issues in vector ptrace kselftest (Andy)
Yong-Xuan Wang (2): riscv: ptrace: Optimize the allocation of vector regset selftests: riscv: Add test for the Vector ptrace interface
arch/riscv/include/asm/vector.h | 1 + arch/riscv/kernel/ptrace.c | 24 +++- arch/riscv/kernel/vector.c | 2 + tools/testing/selftests/riscv/vector/Makefile | 5 +- .../selftests/riscv/vector/vstate_ptrace.c | 134 ++++++++++++++++++ 5 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c
The vector regset uses the maximum possible vlen value to estimate the .n field. But not all the hardwares support the maximum vlen. Linux might wastes time to prepare a large memory buffer(about 2^6 pages) for the vector regset.
The regset can only copy vector registers when the process are using vector. Add .active callback and determine the n field of vector regset in riscv_v_setup_ctx_cache() doesn't affect the ptrace syscall and coredump. It can avoid oversized allocations and better matches real hardware limits.
Signed-off-by: Yong-Xuan Wang yongxuan.wang@sifive.com Reviewed-by: Greentime Hu greentime.hu@sifive.com Reviewed-by: Andy Chiu andybnac@gmail.com --- arch/riscv/include/asm/vector.h | 1 + arch/riscv/kernel/ptrace.c | 24 +++++++++++++++++++++--- arch/riscv/kernel/vector.c | 2 ++ 3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index b61786d43c20..e7aa449368ad 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -51,6 +51,7 @@ void put_cpu_vector_context(void); void riscv_v_thread_free(struct task_struct *tsk); void __init riscv_v_setup_ctx_cache(void); void riscv_v_thread_alloc(struct task_struct *tsk); +void __init update_regset_vector_info(unsigned long size);
static inline u32 riscv_v_flags(void) { diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c index 8e86305831ea..e6272d74572f 100644 --- a/arch/riscv/kernel/ptrace.c +++ b/arch/riscv/kernel/ptrace.c @@ -153,6 +153,17 @@ static int riscv_vr_set(struct task_struct *target, 0, riscv_v_vsize); return ret; } + +static int riscv_vr_active(struct task_struct *target, const struct user_regset *regset) +{ + if (!(has_vector() || has_xtheadvector())) + return -ENODEV; + + if (!riscv_v_vstate_query(task_pt_regs(target))) + return 0; + + return regset->n; +} #endif
#ifdef CONFIG_RISCV_ISA_SUPM @@ -184,7 +195,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target, } #endif
-static const struct user_regset riscv_user_regset[] = { +static struct user_regset riscv_user_regset[] __ro_after_init = { [REGSET_X] = { USER_REGSET_NOTE_TYPE(PRSTATUS), .n = ELF_NGREG, @@ -207,11 +218,10 @@ static const struct user_regset riscv_user_regset[] = { [REGSET_V] = { USER_REGSET_NOTE_TYPE(RISCV_VECTOR), .align = 16, - .n = ((32 * RISCV_MAX_VLENB) + - sizeof(struct __riscv_v_regset_state)) / sizeof(__u32), .size = sizeof(__u32), .regset_get = riscv_vr_get, .set = riscv_vr_set, + .active = riscv_vr_active, }, #endif #ifdef CONFIG_RISCV_ISA_SUPM @@ -233,6 +243,14 @@ static const struct user_regset_view riscv_user_native_view = { .n = ARRAY_SIZE(riscv_user_regset), };
+#ifdef CONFIG_RISCV_ISA_V +void __init update_regset_vector_info(unsigned long size) +{ + riscv_user_regset[REGSET_V].n = (size + sizeof(struct __riscv_v_regset_state)) / + sizeof(__u32); +} +#endif + struct pt_regs_offset { const char *name; int offset; diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c index 901e67adf576..3ed071dab9d8 100644 --- a/arch/riscv/kernel/vector.c +++ b/arch/riscv/kernel/vector.c @@ -66,6 +66,8 @@ void __init riscv_v_setup_ctx_cache(void) if (!(has_vector() || has_xtheadvector())) return;
+ update_regset_vector_info(riscv_v_vsize); + riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx", riscv_v_vsize, 16, SLAB_PANIC, 0, riscv_v_vsize, NULL);
Add a test case that does some basic verification of the Vector ptrace interface. This forks a child process then using ptrace to inspect and manipulate the v31 register of the child.
Signed-off-by: Yong-Xuan Wang yongxuan.wang@sifive.com Reviewed-by: Andy Chiu andybnac@gmail.com --- tools/testing/selftests/riscv/vector/Makefile | 5 +- .../selftests/riscv/vector/vstate_ptrace.c | 134 ++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c
diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile index 6f7497f4e7b3..2c2a33fc083e 100644 --- a/tools/testing/selftests/riscv/vector/Makefile +++ b/tools/testing/selftests/riscv/vector/Makefile @@ -2,7 +2,7 @@ # Copyright (C) 2021 ARM Limited # Originally tools/testing/arm64/abi/Makefile
-TEST_GEN_PROGS := v_initval vstate_prctl +TEST_GEN_PROGS := v_initval vstate_prctl vstate_ptrace TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
include ../../lib.mk @@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \ -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc + +$(OUTPUT)/vstate_ptrace: vstate_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ diff --git a/tools/testing/selftests/riscv/vector/vstate_ptrace.c b/tools/testing/selftests/riscv/vector/vstate_ptrace.c new file mode 100644 index 000000000000..1479abc0c9cb --- /dev/null +++ b/tools/testing/selftests/riscv/vector/vstate_ptrace.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stdio.h> +#include <stdlib.h> +#include <asm/ptrace.h> +#include <linux/elf.h> +#include <sys/ptrace.h> +#include <sys/uio.h> +#include <sys/wait.h> +#include "../../kselftest.h" +#include "v_helpers.h" + +int parent_set_val, child_set_val; + +static long do_ptrace(enum __ptrace_request op, pid_t pid, long type, size_t size, void *data) +{ + struct iovec v_iovec = { + .iov_len = size, + .iov_base = data + }; + + return ptrace(op, pid, type, &v_iovec); +} + +static int do_child(void) +{ + int out; + + if (ptrace(PTRACE_TRACEME, -1, NULL, NULL)) { + ksft_perror("PTRACE_TRACEME failed\n"); + return EXIT_FAILURE; + } + + asm volatile (".option push\n\t" + ".option arch, +v\n\t" + ".option norvc\n\t" + "vsetivli x0, 1, e32, m1, ta, ma\n\t" + "vmv.s.x v31, %[in]\n\t" + "ebreak\n\t" + "vmv.x.s %[out], v31\n\t" + ".option pop\n\t" + : [out] "=r" (out) + : [in] "r" (child_set_val)); + + if (out != parent_set_val) + return EXIT_FAILURE; + + return EXIT_SUCCESS; +} + +static void do_parent(pid_t child) +{ + int status; + void *data = NULL; + + /* Attach to the child */ + while (waitpid(child, &status, 0)) { + if (WIFEXITED(status)) { + ksft_test_result(WEXITSTATUS(status) == 0, "SETREGSET vector\n"); + goto out; + } else if (WIFSTOPPED(status) && (WSTOPSIG(status) == SIGTRAP)) { + size_t size; + void *data, *v31; + struct __riscv_v_regset_state *v_regset_hdr; + struct user_regs_struct *gpreg; + + size = sizeof(*v_regset_hdr); + data = malloc(size); + if (!data) + goto out; + v_regset_hdr = (struct __riscv_v_regset_state *)data; + + if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data)) + goto out; + + ksft_print_msg("vlenb %ld\n", v_regset_hdr->vlenb); + data = realloc(data, size + v_regset_hdr->vlenb * 32); + if (!data) + goto out; + v_regset_hdr = (struct __riscv_v_regset_state *)data; + v31 = (void *)(data + size + v_regset_hdr->vlenb * 31); + size += v_regset_hdr->vlenb * 32; + + if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data)) + goto out; + + ksft_test_result(*(int *)v31 == child_set_val, "GETREGSET vector\n"); + + *(int *)v31 = parent_set_val; + if (do_ptrace(PTRACE_SETREGSET, child, NT_RISCV_VECTOR, size, data)) + goto out; + + /* move the pc forward */ + size = sizeof(*gpreg); + data = realloc(data, size); + gpreg = (struct user_regs_struct *)data; + + if (do_ptrace(PTRACE_GETREGSET, child, NT_PRSTATUS, size, data)) + goto out; + + gpreg->pc += 4; + if (do_ptrace(PTRACE_SETREGSET, child, NT_PRSTATUS, size, data)) + goto out; + } + + ptrace(PTRACE_CONT, child, NULL, NULL); + } + +out: + free(data); +} + +int main(void) +{ + pid_t child; + + ksft_set_plan(2); + if (!is_vector_supported() && !is_xtheadvector_supported()) + ksft_exit_skip("Vector not supported\n"); + + srandom(getpid()); + parent_set_val = rand(); + child_set_val = rand(); + + child = fork(); + if (child < 0) + ksft_exit_fail_msg("Fork failed %d\n", child); + + if (!child) + return do_child(); + + do_parent(child); + + ksft_finished(); +}
linux-kselftest-mirror@lists.linaro.org
-
Yong-Xuan Wang