This test checks that orig_a0 allows a syscall argument to be modified,
and that changing a0 does not change the syscall argument.
Co-developed-by: Quan Zhou <zhouquan@xxxxxxxxxxx>
Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
---
arch/riscv/kernel/ptrace.c | 2 +-
tools/testing/selftests/riscv/abi/.gitignore | 1 +
tools/testing/selftests/riscv/abi/Makefile | 5 +-
tools/testing/selftests/riscv/abi/ptrace.c | 133 +++++++++++++++++++
4 files changed, 139 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/riscv/abi/ptrace.c
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index faa46de90003..025c22894d32 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -197,7 +197,7 @@ static int riscv_orig_a0_set(struct task_struct *target,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
{
- int orig_a0 = task_pt_regs(target)->orig_a0;
+ unsigned long orig_a0 = task_pt_regs(target)->orig_a0;
int ret;
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &orig_a0, 0, -1);
diff --git a/tools/testing/selftests/riscv/abi/.gitignore b/tools/testing/selftests/riscv/abi/.gitignore
index b38358f91c4d..378c605919a3 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 ed82ff9c664e..3f74d059dfdc 100644
--- a/tools/testing/selftests/riscv/abi/Makefile
+++ b/tools/testing/selftests/riscv/abi/Makefile
@@ -2,9 +2,12 @@
CFLAGS += -I$(top_srcdir)/tools/include
-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 000000000000..1c3ce40d6a34
--- /dev/null
+++ b/tools/testing/selftests/riscv/abi/ptrace.c
@@ -0,0 +1,133 @@
+// 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 <asm/ptrace.h>
+
+#include "../../kselftest_harness.h"
+
+#define ORIG_A0_MODIFY 0x01
+#define A0_MODIFY 0x02
+#define A0_OLD 0x03
+#define A0_NEW 0x04
+
+#define perr_and_exit(fmt, ...) \
+ ({ \
+ char buf[256]; \
+ snprintf(buf, sizeof(buf), "%s:%d:" fmt ": %m\n", \
+ __func__, __LINE__, ##__VA_ARGS__); \
+ perror(buf); \
+ exit(-1); \
+ })
+
+static inline void resume_and_wait_tracee(pid_t pid, int flag)
+{
+ int status;
+
+ if (ptrace(flag, pid, 0, 0))
+ perr_and_exit("failed to resume the tracee %d\n", pid);
+
+ if (waitpid(pid, &status, 0) != pid)
+ perr_and_exit("failed to wait for the tracee %d\n", pid);
+}
+
+static void ptrace_test(int opt, int *result)
+{
+ int status;
+ 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),
+ };
+
+ pid = fork();
+ if (pid == 0) {
+ /* Mark oneself being traced */
+ long val = ptrace(PTRACE_TRACEME, 0, 0, 0);
+ if (val)
+ perr_and_exit("failed to request for tracer to trace me: %ld\n", val);
+
+ kill(getpid(), SIGSTOP);
+
+ /* Perform exit syscall that will be intercepted */
+ exit(A0_OLD);
+ }
+
+ if (pid < 0)
+ exit(1);
+
+ if (waitpid(pid, &status, 0) != pid)
+ perr_and_exit("failed to wait for the tracee %d\n", pid);
+
+ /* Stop at the entry point of the syscall */
+ resume_and_wait_tracee(pid, PTRACE_SYSCALL);
+
+ /* Check tracee regs before the syscall */
+ if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))
+ perr_and_exit("failed to get tracee registers\n");
+ if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov))
+ perr_and_exit("failed to get tracee registers\n");
+ if (orig_a0 != A0_OLD)
+ perr_and_exit("unexpected orig_a0: 0x%lx\n", orig_a0);
+
+ /* 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_RISCV_ORIG_A0, &a0_iov))
+ perr_and_exit("failed to set tracee registers\n");
+
+ /* Resume the tracee */
+ ptrace(PTRACE_CONT, pid, 0, 0);
+ if (waitpid(pid, &status, 0) != pid)
+ perr_and_exit("failed to wait for the tracee\n");
+
+ *result = WEXITSTATUS(status);
+}
+
+TEST(ptrace_modify_a0)
+{
+ int result;
+
+ ptrace_test(A0_MODIFY, &result);
+
+ /* The modification of a0 cannot affect the first argument of the syscall */
+ EXPECT_EQ(A0_OLD, result);
+}
+
+TEST(ptrace_modify_orig_a0)
+{
+ int result;
+
+ ptrace_test(ORIG_A0_MODIFY, &result);
+
+ /* Only modify orig_a0 to change the first argument of the syscall */
+ EXPECT_EQ(A0_NEW, result);
+}
+
+TEST_HARNESS_MAIN
--
2.34.1
>
> 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@xxxxxxxxx/
> Signed-off-by: Celeste Liu <uwu@xxxxxxxxxxxxxxxxx>
> ---
> arch/riscv/kernel/ptrace.c | 33 +++++++++++++++++++++++++++++++++
> include/uapi/linux/elf.h | 1 +
> 2 files changed, 34 insertions(+)
>
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index ea67e9fb7a583683b922fe2c017ea61f3bc848db..faa46de9000376eb445a32d43a40210d7b846844 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,30 @@ 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)
> +{
> + int orig_a0 = task_pt_regs(target)->orig_a0;
The testcase above highlights that this should be of type "unsigned
long" instead of int! Otherwise 64-bit systems will only be able to set
the first 32 bits (as Björn pointed out in the other thread) :)
This issue was found because the test case tries to set all 64 bits and
succeeds, but the extra bits corrupt the stack. Maybe the code here
should enforce that the count is equal to the size of an unsigned long?
Fortunately the extra bits ended up in the stack so it was determined to
be corrupted, but I suppose that will not necessarily always be the case
depending on kernel compiler optimizations and user_regset_copyin()
could end up overwritting other data in this function undetected.
- Charlie
> + 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 ret;
> +}
> +
> +
> static const struct user_regset riscv_user_regset[] = {
> [REGSET_X] = {
> .core_note_type = NT_PRSTATUS,
> @@ -224,6 +249,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 */
>
> ---
> base-commit: 0e287d31b62bb53ad81d5e59778384a40f8b6f56
> change-id: 20241201-riscv-new-regset-d529b952ad0d
>
> Best regards,
> --
> Celeste Liu <uwu@xxxxxxxxxxxxxxxxx>
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv
