The orig_a0 is missing in struct user_regs_struct of riscv, and there is no way to add it without breaking UAPI. (See Link tag below)
Like NT_ARM_SYSTEM_CALL do, we add a new regset name NT_RISCV_ORIG_A0 to access original a0 register from userspace via ptrace API.
Link: https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/
Signed-off-by: Celeste Liu uwu@coelacanthus.name --- Changes in v5: - Fix wrong usage in selftests. - Link to v4: https://lore.kernel.org/r/20241226-riscv-new-regset-v4-0-4496a29d0436@coelac...
Changes in v4: - Fix a copy paste error in selftest. (Forget to commit...) - Link to v3: https://lore.kernel.org/r/20241226-riscv-new-regset-v3-0-f5b96465826b@coelac...
Changes in v3: - Use return 0 directly for readability. - Fix test for modify a0. - Add Fixes: tag - Remove useless Cc: stable. - Selftest will check both a0 and orig_a0, but depends on the correctness of PTRACE_GET_SYSCALL_INFO. - Link to v2: https://lore.kernel.org/r/20241203-riscv-new-regset-v2-0-d37da8c0cba6@coelac...
Changes in v2: - Fix integer width. - Add selftest. - Link to v1: https://lore.kernel.org/r/20241201-riscv-new-regset-v1-1-c83c58abcc7b@coelac...
--- Celeste Liu (2): riscv/ptrace: add new regset to access original a0 register riscv: selftests: Add a ptrace test to verify a0 and orig_a0 access
arch/riscv/kernel/ptrace.c | 32 +++++ include/uapi/linux/elf.h | 1 + tools/testing/selftests/riscv/abi/.gitignore | 1 + tools/testing/selftests/riscv/abi/Makefile | 6 +- tools/testing/selftests/riscv/abi/ptrace.c | 201 +++++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 1 deletion(-) --- base-commit: 0e287d31b62bb53ad81d5e59778384a40f8b6f56 change-id: 20241201-riscv-new-regset-d529b952ad0d
Best regards,
The orig_a0 is missing in struct user_regs_struct of riscv, and there is no way to add it without breaking UAPI. (See Link tag below)
Like NT_ARM_SYSTEM_CALL do, we add a new regset name NT_RISCV_ORIG_A0 to access original a0 register from userspace via ptrace API.
Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API") Link: https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/ Cc: stable@vger.kernel.org Reviewed-by: Björn Töpel bjorn@rivosinc.com Signed-off-by: Celeste Liu uwu@coelacanthus.name --- arch/riscv/kernel/ptrace.c | 32 ++++++++++++++++++++++++++++++++ include/uapi/linux/elf.h | 1 + 2 files changed, 33 insertions(+)
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c index ea67e9fb7a583683b922fe2c017ea61f3bc848db..ef9ab74c8575a5c440155973b1c625c06a867c97 100644 --- a/arch/riscv/kernel/ptrace.c +++ b/arch/riscv/kernel/ptrace.c @@ -31,6 +31,7 @@ enum riscv_regset { #ifdef CONFIG_RISCV_ISA_SUPM REGSET_TAGGED_ADDR_CTRL, #endif + REGSET_ORIG_A0, };
static int riscv_gpr_get(struct task_struct *target, @@ -184,6 +185,29 @@ static int tagged_addr_ctrl_set(struct task_struct *target, } #endif
+static int riscv_orig_a0_get(struct task_struct *target, + const struct user_regset *regset, + struct membuf to) +{ + return membuf_store(&to, task_pt_regs(target)->orig_a0); +} + +static int riscv_orig_a0_set(struct task_struct *target, + const struct user_regset *regset, + unsigned int pos, unsigned int count, + const void *kbuf, const void __user *ubuf) +{ + unsigned long orig_a0 = task_pt_regs(target)->orig_a0; + int ret; + + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &orig_a0, 0, -1); + if (ret) + return ret; + + task_pt_regs(target)->orig_a0 = orig_a0; + return 0; +} + static const struct user_regset riscv_user_regset[] = { [REGSET_X] = { .core_note_type = NT_PRSTATUS, @@ -224,6 +248,14 @@ static const struct user_regset riscv_user_regset[] = { .set = tagged_addr_ctrl_set, }, #endif + [REGSET_ORIG_A0] = { + .core_note_type = NT_RISCV_ORIG_A0, + .n = 1, + .size = sizeof(elf_greg_t), + .align = sizeof(elf_greg_t), + .regset_get = riscv_orig_a0_get, + .set = riscv_orig_a0_set, + }, };
static const struct user_regset_view riscv_user_native_view = { diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index b44069d29cecc0f9de90ee66bfffd2137f4275a8..390060229601631da2fb27030d9fa2142e676c14 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -452,6 +452,7 @@ typedef struct elf64_shdr { #define NT_RISCV_CSR 0x900 /* RISC-V Control and Status Registers */ #define NT_RISCV_VECTOR 0x901 /* RISC-V vector registers */ #define NT_RISCV_TAGGED_ADDR_CTRL 0x902 /* RISC-V tagged address control (prctl()) */ +#define NT_RISCV_ORIG_A0 0x903 /* RISC-V original a0 register */ #define NT_LOONGARCH_CPUCFG 0xa00 /* LoongArch CPU config registers */ #define NT_LOONGARCH_CSR 0xa01 /* LoongArch control and status registers */ #define NT_LOONGARCH_LSX 0xa02 /* LoongArch Loongson SIMD Extension registers */
This test checks that orig_a0 and a0 can be modified and accessed independently.
Co-developed-by: Quan Zhou zhouquan@iscas.ac.cn Signed-off-by: Quan Zhou zhouquan@iscas.ac.cn Co-developed-by: Charlie Jenkins charlie@rivosinc.com Signed-off-by: Charlie Jenkins charlie@rivosinc.com Reviewed-by: Björn Töpel bjorn@rivosinc.com Signed-off-by: Celeste Liu uwu@coelacanthus.name --- tools/testing/selftests/riscv/abi/.gitignore | 1 + tools/testing/selftests/riscv/abi/Makefile | 6 +- tools/testing/selftests/riscv/abi/ptrace.c | 201 +++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/riscv/abi/.gitignore b/tools/testing/selftests/riscv/abi/.gitignore index b38358f91c4d2240ae64892871d9ca98bda1ae58..378c605919a3b3d58eec2701eb7af430cfe315d6 100644 --- a/tools/testing/selftests/riscv/abi/.gitignore +++ b/tools/testing/selftests/riscv/abi/.gitignore @@ -1 +1,2 @@ pointer_masking +ptrace diff --git a/tools/testing/selftests/riscv/abi/Makefile b/tools/testing/selftests/riscv/abi/Makefile index ed82ff9c664e7eb3f760cbab81fb957ff72579c5..359a082c88a401883fb3776b35e4dacf69beaaaa 100644 --- a/tools/testing/selftests/riscv/abi/Makefile +++ b/tools/testing/selftests/riscv/abi/Makefile @@ -1,10 +1,14 @@ # SPDX-License-Identifier: GPL-2.0
CFLAGS += -I$(top_srcdir)/tools/include +CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := pointer_masking +TEST_GEN_PROGS := pointer_masking ptrace
include ../../lib.mk
$(OUTPUT)/pointer_masking: pointer_masking.c $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ + +$(OUTPUT)/ptrace: ptrace.c + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ diff --git a/tools/testing/selftests/riscv/abi/ptrace.c b/tools/testing/selftests/riscv/abi/ptrace.c new file mode 100644 index 0000000000000000000000000000000000000000..f1a0458adccdd040bfaa350e2e8d98b1ef34c0ad --- /dev/null +++ b/tools/testing/selftests/riscv/abi/ptrace.c @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <signal.h> +#include <errno.h> +#include <sys/types.h> +#include <sys/ptrace.h> +#include <sys/stat.h> +#include <sys/user.h> +#include <sys/wait.h> +#include <sys/uio.h> +#include <linux/elf.h> +#include <linux/unistd.h> +#include <linux/ptrace.h> +#include <asm/ptrace.h> + +#include "../../kselftest_harness.h" + +#ifndef sizeof_field +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#endif +#ifndef offsetofend +#define offsetofend(TYPE, MEMBER) \ + (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER)) +#endif + + +#define ORIG_A0_MODIFY 0x01 +#define A0_MODIFY 0x02 +#define A0_OLD 0xbadbeefbeeff +#define A0_NEW 0xffeebfeebdab + + +struct a0_regs { + __s64 orig_a0; + __u64 a0; +}; + +#define perr_and_exit(fmt, ...) \ + ({ \ + char buf[256]; \ + snprintf(buf, sizeof(buf), "%s:%d:" fmt ": %m\n", \ + __func__, __LINE__, ##__VA_ARGS__); \ + ksft_exit_fail_perror(buf); \ + }) + +static void ptrace_test(int opt, struct a0_regs result[]) +{ + int status; + long rc; + pid_t pid; + struct user_regs_struct regs; + struct iovec iov = { + .iov_base = ®s, + .iov_len = sizeof(regs), + }; + + unsigned long orig_a0; + struct iovec a0_iov = { + .iov_base = &orig_a0, + .iov_len = sizeof(orig_a0), + }; + struct ptrace_syscall_info syscall_info = { + .op = 0xff, + }; + const unsigned int expected_sci_entry_size = + offsetofend(struct ptrace_syscall_info, entry.args); + const unsigned int expected_sci_exit_size = + offsetofend(struct ptrace_syscall_info, exit.is_error); + + pid = fork(); + if (pid == 0) { + /* Mark oneself being traced */ + long val = ptrace(PTRACE_TRACEME, 0, 0, 0); + + if (val < 0) + perr_and_exit("failed to request for tracer to trace me: %ld", val); + + kill(getpid(), SIGSTOP); + + /* Perform chdir syscall that will be intercepted */ + syscall(__NR_chdir, A0_OLD); + + exit(0); + } + + if (pid < 0) + ksft_exit_fail_perror("failed to fork"); + + for (int i = 0; i < 3; i++) { + if (waitpid(pid, &status, 0) != pid) + perr_and_exit("failed to wait for the tracee %d", pid); + if (WIFSTOPPED(status)) { + switch (WSTOPSIG(status)) { + case SIGSTOP: + if (ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_TRACESYSGOOD) < 0) + perr_and_exit("failed to set PTRACE_O_TRACESYSGOOD"); + break; + case SIGTRAP|0x80: + /* Modify twice so GET_SYSCALL_INFO get modified a0 and orig_a0 */ + if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov)) + perr_and_exit("failed to get tracee registers"); + if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov)) + perr_and_exit("failed to get tracee registers"); + + /* Modify a0/orig_a0 for the syscall */ + switch (opt) { + case A0_MODIFY: + regs.a0 = A0_NEW; + break; + case ORIG_A0_MODIFY: + orig_a0 = A0_NEW; + break; + } + + if (ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov)) + perr_and_exit("failed to set tracee registers"); + if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov)) + perr_and_exit("failed to set tracee registers"); + switch (i) { + case 1: + /* Stop at the beginning of syscall */ + rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, + sizeof(syscall_info), &syscall_info); + if (rc < 0) + perr_and_exit("failed to get syscall info of entry"); + if (rc < expected_sci_entry_size + || syscall_info.op != PTRACE_SYSCALL_INFO_ENTRY) + perr_and_exit("stop position of entry mismatched"); + result[0].orig_a0 = syscall_info.entry.args[0]; + break; + + case 2: + /* Stop at the end of syscall */ + rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, + sizeof(syscall_info), &syscall_info); + if (rc < 0) + perr_and_exit("failed to get syscall info of entry"); + if (rc < expected_sci_exit_size + || syscall_info.op != PTRACE_SYSCALL_INFO_EXIT) + perr_and_exit("stop position of exit mismatched"); + result[0].a0 = syscall_info.exit.rval; + + if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov)) + perr_and_exit("failed to get tracee registers"); + result[1].a0 = regs.a0; + if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, + &a0_iov)) + perr_and_exit("failed to get tracee registers"); + result[1].orig_a0 = orig_a0; + } + } + if (ptrace(PTRACE_SYSCALL, pid, 0, 0) < 0) + perr_and_exit("failed to resume tracee"); + } + } + + /* Resume the tracee */ + ptrace(PTRACE_CONT, pid, 0, 0); + if (waitpid(pid, &status, 0) != pid) + perr_and_exit("failed to wait for the tracee"); + +} + +TEST(ptrace_access_a0) +{ + struct a0_regs result[2]; + + ptrace_test(A0_MODIFY, result); + + /* Verify PTRACE_SETREGSET */ + /* The modification of a0 cannot affect the first argument of the syscall */ + EXPECT_EQ(A0_OLD, result[0].orig_a0); + EXPECT_EQ(A0_NEW, result[0].a0); + + /* Verify PTRACE_GETREGSET */ + EXPECT_EQ(result[1].orig_a0, result[0].orig_a0); + EXPECT_EQ(result[1].a0, result[0].a0); +} + +TEST(ptrace_access_orig_a0) +{ + struct a0_regs result[2]; + + ptrace_test(ORIG_A0_MODIFY, result); + + /* Verify PTRACE_SETREGSET */ + /* Only modify orig_a0 to change the first argument of the syscall */ + EXPECT_EQ(A0_NEW, result[0].orig_a0); + /* a0 will keep default value, orig_a0 or -ENOSYS, depends on internal. */ + EXPECT_NE(A0_NEW, result[0].a0); + + /* Verify PTRACE_GETREGSET */ + EXPECT_EQ(result[1].orig_a0, result[0].orig_a0); + EXPECT_EQ(result[1].a0, result[0].a0); +} + +TEST_HARNESS_MAIN
On Wed, Jan 15, 2025 at 04:24:59AM +0800, Celeste Liu wrote:
This test checks that orig_a0 and a0 can be modified and accessed independently.
Co-developed-by: Quan Zhou zhouquan@iscas.ac.cn Signed-off-by: Quan Zhou zhouquan@iscas.ac.cn Co-developed-by: Charlie Jenkins charlie@rivosinc.com Signed-off-by: Charlie Jenkins charlie@rivosinc.com Reviewed-by: Björn Töpel bjorn@rivosinc.com Signed-off-by: Celeste Liu uwu@coelacanthus.name
tools/testing/selftests/riscv/abi/.gitignore | 1 + tools/testing/selftests/riscv/abi/Makefile | 6 +- tools/testing/selftests/riscv/abi/ptrace.c | 201 +++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/riscv/abi/.gitignore b/tools/testing/selftests/riscv/abi/.gitignore index b38358f91c4d2240ae64892871d9ca98bda1ae58..378c605919a3b3d58eec2701eb7af430cfe315d6 100644 --- a/tools/testing/selftests/riscv/abi/.gitignore +++ b/tools/testing/selftests/riscv/abi/.gitignore @@ -1 +1,2 @@ pointer_masking +ptrace diff --git a/tools/testing/selftests/riscv/abi/Makefile b/tools/testing/selftests/riscv/abi/Makefile index ed82ff9c664e7eb3f760cbab81fb957ff72579c5..359a082c88a401883fb3776b35e4dacf69beaaaa 100644 --- a/tools/testing/selftests/riscv/abi/Makefile +++ b/tools/testing/selftests/riscv/abi/Makefile @@ -1,10 +1,14 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS += -I$(top_srcdir)/tools/include +CFLAGS += $(KHDR_INCLUDES) -TEST_GEN_PROGS := pointer_masking +TEST_GEN_PROGS := pointer_masking ptrace include ../../lib.mk $(OUTPUT)/pointer_masking: pointer_masking.c $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
+$(OUTPUT)/ptrace: ptrace.c
- $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/abi/ptrace.c b/tools/testing/selftests/riscv/abi/ptrace.c new file mode 100644 index 0000000000000000000000000000000000000000..f1a0458adccdd040bfaa350e2e8d98b1ef34c0ad --- /dev/null +++ b/tools/testing/selftests/riscv/abi/ptrace.c @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <signal.h> +#include <errno.h> +#include <sys/types.h> +#include <sys/ptrace.h> +#include <sys/stat.h> +#include <sys/user.h> +#include <sys/wait.h> +#include <sys/uio.h> +#include <linux/elf.h> +#include <linux/unistd.h> +#include <linux/ptrace.h> +#include <asm/ptrace.h>
+#include "../../kselftest_harness.h"
+#ifndef sizeof_field +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#endif +#ifndef offsetofend +#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
+#endif
I think this is the sixth test to define these. We should copy include/linux/stddef.h into tools/include. We already have tools/include/uapi/linux/stddef.h with __struct_group and __DECLARE_FLEX_ARRAY, so I think it should just work.
+#define ORIG_A0_MODIFY 0x01 +#define A0_MODIFY 0x02 +#define A0_OLD 0xbadbeefbeeff +#define A0_NEW 0xffeebfeebdab
+struct a0_regs {
- __s64 orig_a0;
- __u64 a0;
+};
+#define perr_and_exit(fmt, ...) \
- ({ \
char buf[256]; \snprintf(buf, sizeof(buf), "%s:%d:" fmt ": %m\n", \__func__, __LINE__, ##__VA_ARGS__); \ksft_exit_fail_perror(buf); \- })
+static void ptrace_test(int opt, struct a0_regs result[]) +{
- int status;
- long rc;
- pid_t pid;
- struct user_regs_struct regs;
- struct iovec iov = {
.iov_base = ®s,.iov_len = sizeof(regs),- };
- unsigned long orig_a0;
- struct iovec a0_iov = {
.iov_base = &orig_a0,.iov_len = sizeof(orig_a0),- };
- struct ptrace_syscall_info syscall_info = {
.op = 0xff,- };
- const unsigned int expected_sci_entry_size =
offsetofend(struct ptrace_syscall_info, entry.args);- const unsigned int expected_sci_exit_size =
offsetofend(struct ptrace_syscall_info, exit.is_error);- pid = fork();
- if (pid == 0) {
/* Mark oneself being traced */long val = ptrace(PTRACE_TRACEME, 0, 0, 0);if (val < 0)perr_and_exit("failed to request for tracer to trace me: %ld", val);kill(getpid(), SIGSTOP);/* Perform chdir syscall that will be intercepted */syscall(__NR_chdir, A0_OLD);exit(0);- }
- if (pid < 0)
ksft_exit_fail_perror("failed to fork");- for (int i = 0; i < 3; i++) {
if (waitpid(pid, &status, 0) != pid)perr_and_exit("failed to wait for the tracee %d", pid);if (WIFSTOPPED(status)) {switch (WSTOPSIG(status)) {case SIGSTOP:if (ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_TRACESYSGOOD) < 0)perr_and_exit("failed to set PTRACE_O_TRACESYSGOOD");break;case SIGTRAP|0x80:/* Modify twice so GET_SYSCALL_INFO get modified a0 and orig_a0 */if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to get tracee registers");if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov))perr_and_exit("failed to get tracee registers");/* Modify a0/orig_a0 for the syscall */switch (opt) {case A0_MODIFY:regs.a0 = A0_NEW;break;case ORIG_A0_MODIFY:orig_a0 = A0_NEW;break;}if (ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to set tracee registers");if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov))perr_and_exit("failed to set tracee registers");switch (i) {case 1:/* Stop at the beginning of syscall */rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid,sizeof(syscall_info), &syscall_info);if (rc < 0)perr_and_exit("failed to get syscall info of entry");if (rc < expected_sci_entry_size|| syscall_info.op != PTRACE_SYSCALL_INFO_ENTRY)perr_and_exit("stop position of entry mismatched");result[0].orig_a0 = syscall_info.entry.args[0];break;case 2:/* Stop at the end of syscall */rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid,sizeof(syscall_info), &syscall_info);if (rc < 0)perr_and_exit("failed to get syscall info of entry");if (rc < expected_sci_exit_size|| syscall_info.op != PTRACE_SYSCALL_INFO_EXIT)perr_and_exit("stop position of exit mismatched");result[0].a0 = syscall_info.exit.rval;if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to get tracee registers");result[1].a0 = regs.a0;if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0,&a0_iov))perr_and_exit("failed to get tracee registers");result[1].orig_a0 = orig_a0;}}if (ptrace(PTRACE_SYSCALL, pid, 0, 0) < 0)perr_and_exit("failed to resume tracee");}- }
- /* Resume the tracee */
- ptrace(PTRACE_CONT, pid, 0, 0);
- if (waitpid(pid, &status, 0) != pid)
perr_and_exit("failed to wait for the tracee");+}
+TEST(ptrace_access_a0) +{
- struct a0_regs result[2];
- ptrace_test(A0_MODIFY, result);
- /* Verify PTRACE_SETREGSET */
- /* The modification of a0 cannot affect the first argument of the syscall */
- EXPECT_EQ(A0_OLD, result[0].orig_a0);
- EXPECT_EQ(A0_NEW, result[0].a0);
- /* Verify PTRACE_GETREGSET */
- EXPECT_EQ(result[1].orig_a0, result[0].orig_a0);
- EXPECT_EQ(result[1].a0, result[0].a0);
+}
+TEST(ptrace_access_orig_a0) +{
- struct a0_regs result[2];
- ptrace_test(ORIG_A0_MODIFY, result);
- /* Verify PTRACE_SETREGSET */
- /* Only modify orig_a0 to change the first argument of the syscall */
- EXPECT_EQ(A0_NEW, result[0].orig_a0);
- /* a0 will keep default value, orig_a0 or -ENOSYS, depends on internal. */
- EXPECT_NE(A0_NEW, result[0].a0);
I don't understand this test. Why don't we know what to expect? Also, the comment says orig_a0 is an option for the value, but then we don't expect it to be A0_NEW, even though we expect orig_a0 to be A0_NEW?
- /* Verify PTRACE_GETREGSET */
- EXPECT_EQ(result[1].orig_a0, result[0].orig_a0);
- EXPECT_EQ(result[1].a0, result[0].a0);
+}
+TEST_HARNESS_MAIN
-- 2.48.0
Other than the two comments/questions, this test looks great.
Thanks, drew
On 2025-01-15 17:14, Andrew Jones wrote:
On Wed, Jan 15, 2025 at 04:24:59AM +0800, Celeste Liu wrote:
This test checks that orig_a0 and a0 can be modified and accessed independently.
Co-developed-by: Quan Zhou zhouquan@iscas.ac.cn Signed-off-by: Quan Zhou zhouquan@iscas.ac.cn Co-developed-by: Charlie Jenkins charlie@rivosinc.com Signed-off-by: Charlie Jenkins charlie@rivosinc.com Reviewed-by: Björn Töpel bjorn@rivosinc.com Signed-off-by: Celeste Liu uwu@coelacanthus.name
tools/testing/selftests/riscv/abi/.gitignore | 1 + tools/testing/selftests/riscv/abi/Makefile | 6 +- tools/testing/selftests/riscv/abi/ptrace.c | 201 +++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/riscv/abi/.gitignore b/tools/testing/selftests/riscv/abi/.gitignore index b38358f91c4d2240ae64892871d9ca98bda1ae58..378c605919a3b3d58eec2701eb7af430cfe315d6 100644 --- a/tools/testing/selftests/riscv/abi/.gitignore +++ b/tools/testing/selftests/riscv/abi/.gitignore @@ -1 +1,2 @@ pointer_masking +ptrace diff --git a/tools/testing/selftests/riscv/abi/Makefile b/tools/testing/selftests/riscv/abi/Makefile index ed82ff9c664e7eb3f760cbab81fb957ff72579c5..359a082c88a401883fb3776b35e4dacf69beaaaa 100644 --- a/tools/testing/selftests/riscv/abi/Makefile +++ b/tools/testing/selftests/riscv/abi/Makefile @@ -1,10 +1,14 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS += -I$(top_srcdir)/tools/include +CFLAGS += $(KHDR_INCLUDES) -TEST_GEN_PROGS := pointer_masking +TEST_GEN_PROGS := pointer_masking ptrace include ../../lib.mk $(OUTPUT)/pointer_masking: pointer_masking.c $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
+$(OUTPUT)/ptrace: ptrace.c
- $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/abi/ptrace.c b/tools/testing/selftests/riscv/abi/ptrace.c new file mode 100644 index 0000000000000000000000000000000000000000..f1a0458adccdd040bfaa350e2e8d98b1ef34c0ad --- /dev/null +++ b/tools/testing/selftests/riscv/abi/ptrace.c @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <signal.h> +#include <errno.h> +#include <sys/types.h> +#include <sys/ptrace.h> +#include <sys/stat.h> +#include <sys/user.h> +#include <sys/wait.h> +#include <sys/uio.h> +#include <linux/elf.h> +#include <linux/unistd.h> +#include <linux/ptrace.h> +#include <asm/ptrace.h>
+#include "../../kselftest_harness.h"
+#ifndef sizeof_field +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#endif +#ifndef offsetofend +#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
+#endif
I think this is the sixth test to define these. We should copy include/linux/stddef.h into tools/include. We already have tools/include/uapi/linux/stddef.h with __struct_group and __DECLARE_FLEX_ARRAY, so I think it should just work.
Agreed. But it may be better to be a separate patchset so we can change those definition in different selftests one pass.
+#define ORIG_A0_MODIFY 0x01 +#define A0_MODIFY 0x02 +#define A0_OLD 0xbadbeefbeeff +#define A0_NEW 0xffeebfeebdab
+struct a0_regs {
- __s64 orig_a0;
- __u64 a0;
+};
+#define perr_and_exit(fmt, ...) \
- ({ \
char buf[256]; \snprintf(buf, sizeof(buf), "%s:%d:" fmt ": %m\n", \__func__, __LINE__, ##__VA_ARGS__); \ksft_exit_fail_perror(buf); \- })
+static void ptrace_test(int opt, struct a0_regs result[]) +{
- int status;
- long rc;
- pid_t pid;
- struct user_regs_struct regs;
- struct iovec iov = {
.iov_base = ®s,.iov_len = sizeof(regs),- };
- unsigned long orig_a0;
- struct iovec a0_iov = {
.iov_base = &orig_a0,.iov_len = sizeof(orig_a0),- };
- struct ptrace_syscall_info syscall_info = {
.op = 0xff,- };
- const unsigned int expected_sci_entry_size =
offsetofend(struct ptrace_syscall_info, entry.args);- const unsigned int expected_sci_exit_size =
offsetofend(struct ptrace_syscall_info, exit.is_error);- pid = fork();
- if (pid == 0) {
/* Mark oneself being traced */long val = ptrace(PTRACE_TRACEME, 0, 0, 0);if (val < 0)perr_and_exit("failed to request for tracer to trace me: %ld", val);kill(getpid(), SIGSTOP);/* Perform chdir syscall that will be intercepted */syscall(__NR_chdir, A0_OLD);exit(0);- }
- if (pid < 0)
ksft_exit_fail_perror("failed to fork");- for (int i = 0; i < 3; i++) {
if (waitpid(pid, &status, 0) != pid)perr_and_exit("failed to wait for the tracee %d", pid);if (WIFSTOPPED(status)) {switch (WSTOPSIG(status)) {case SIGSTOP:if (ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_TRACESYSGOOD) < 0)perr_and_exit("failed to set PTRACE_O_TRACESYSGOOD");break;case SIGTRAP|0x80:/* Modify twice so GET_SYSCALL_INFO get modified a0 and orig_a0 */if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to get tracee registers");if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov))perr_and_exit("failed to get tracee registers");/* Modify a0/orig_a0 for the syscall */switch (opt) {case A0_MODIFY:regs.a0 = A0_NEW;break;case ORIG_A0_MODIFY:orig_a0 = A0_NEW;break;}if (ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to set tracee registers");if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov))perr_and_exit("failed to set tracee registers");switch (i) {case 1:/* Stop at the beginning of syscall */rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid,sizeof(syscall_info), &syscall_info);if (rc < 0)perr_and_exit("failed to get syscall info of entry");if (rc < expected_sci_entry_size|| syscall_info.op != PTRACE_SYSCALL_INFO_ENTRY)perr_and_exit("stop position of entry mismatched");result[0].orig_a0 = syscall_info.entry.args[0];break;case 2:/* Stop at the end of syscall */rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid,sizeof(syscall_info), &syscall_info);if (rc < 0)perr_and_exit("failed to get syscall info of entry");if (rc < expected_sci_exit_size|| syscall_info.op != PTRACE_SYSCALL_INFO_EXIT)perr_and_exit("stop position of exit mismatched");result[0].a0 = syscall_info.exit.rval;if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))perr_and_exit("failed to get tracee registers");result[1].a0 = regs.a0;if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0,&a0_iov))perr_and_exit("failed to get tracee registers");result[1].orig_a0 = orig_a0;}}if (ptrace(PTRACE_SYSCALL, pid, 0, 0) < 0)perr_and_exit("failed to resume tracee");}- }
- /* Resume the tracee */
- ptrace(PTRACE_CONT, pid, 0, 0);
- if (waitpid(pid, &status, 0) != pid)
perr_and_exit("failed to wait for the tracee");+}
+TEST(ptrace_access_a0) +{
- struct a0_regs result[2];
- ptrace_test(A0_MODIFY, result);
- /* Verify PTRACE_SETREGSET */
- /* The modification of a0 cannot affect the first argument of the syscall */
- EXPECT_EQ(A0_OLD, result[0].orig_a0);
- EXPECT_EQ(A0_NEW, result[0].a0);
- /* Verify PTRACE_GETREGSET */
- EXPECT_EQ(result[1].orig_a0, result[0].orig_a0);
- EXPECT_EQ(result[1].a0, result[0].a0);
+}
+TEST(ptrace_access_orig_a0) +{
- struct a0_regs result[2];
- ptrace_test(ORIG_A0_MODIFY, result);
- /* Verify PTRACE_SETREGSET */
- /* Only modify orig_a0 to change the first argument of the syscall */
- EXPECT_EQ(A0_NEW, result[0].orig_a0);
- /* a0 will keep default value, orig_a0 or -ENOSYS, depends on internal. */
- EXPECT_NE(A0_NEW, result[0].a0);
I don't understand this test. Why don't we know what to expect? Also, the comment says orig_a0 is an option for the value, but then we don't expect it to be A0_NEW, even though we expect orig_a0 to be A0_NEW?
The purpose of the test is to ensure that the ORIG_A0_MODIFY operation will not affect the a0 register (So it will not be our A0_NEW). But there is a problem with the comment. It is written for some old wrong code. I will correct the comment.
- /* Verify PTRACE_GETREGSET */
- EXPECT_EQ(result[1].orig_a0, result[0].orig_a0);
- EXPECT_EQ(result[1].a0, result[0].a0);
+}
+TEST_HARNESS_MAIN
-- 2.48.0
Other than the two comments/questions, this test looks great.
Thanks, drew
On Wed, Jan 15, 2025 at 05:41:57PM +0800, Celeste Liu wrote:
On 2025-01-15 17:14, Andrew Jones wrote:
On Wed, Jan 15, 2025 at 04:24:59AM +0800, Celeste Liu wrote:
...
+#ifndef sizeof_field +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#endif +#ifndef offsetofend +#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
+#endif
I think this is the sixth test to define these. We should copy include/linux/stddef.h into tools/include. We already have tools/include/uapi/linux/stddef.h with __struct_group and __DECLARE_FLEX_ARRAY, so I think it should just work.
Agreed. But it may be better to be a separate patchset so we can change those definition in different selftests one pass.
I think a separate "copy stddef.h" patch could be in this series to avoid having to add the defines here. Then, another series can be sent with one patch for each conversion. That said, I'm OK with adding the defines for now and doing the conversion later. I just hope it will actually happen.
Thanks, drew
On 2025-01-15 17:56, Andrew Jones wrote:
On Wed, Jan 15, 2025 at 05:41:57PM +0800, Celeste Liu wrote:
On 2025-01-15 17:14, Andrew Jones wrote:
On Wed, Jan 15, 2025 at 04:24:59AM +0800, Celeste Liu wrote:
...
+#ifndef sizeof_field +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#endif +#ifndef offsetofend +#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
+#endif
I think this is the sixth test to define these. We should copy include/linux/stddef.h into tools/include. We already have tools/include/uapi/linux/stddef.h with __struct_group and __DECLARE_FLEX_ARRAY, so I think it should just work.
Agreed. But it may be better to be a separate patchset so we can change those definition in different selftests one pass.
I think a separate "copy stddef.h" patch could be in this series to avoid having to add the defines here. Then, another series can be sent with one patch for each conversion. That said, I'm OK with adding the defines for now and doing the conversion later. I just hope it will actually happen.
v6 has been sent. The "copy stddef.h" patch has been included. Once this patchset land, I will send another patchset to deal with other usages.
https://lore.kernel.org/lkml/20250115-riscv-new-regset-v6-0-59bfddd33525@coe...
Thanks, drew
linux-kselftest-mirror@lists.linaro.org
-
Andrew Jones -
Celeste Liu