On Fri, Jun 21, 2024 at 02:29:07PM +0800, Quan Zhou wrote:
On 2024/6/20 10:55, Charlie Jenkins wrote:
On Wed, Jun 19, 2024 at 10:01:47AM +0800, zhouquan@xxxxxxxxxxx wrote:
From: Quan Zhou <zhouquan@xxxxxxxxxxx>
This test creates two processes: a tracer and a tracee. The tracer actively
sends a SIGUSR1 signal in user mode to interrupt the read syscall being
executed by the tracee. We will reset a0/orig_a0 and then observe the value
of a0 held by the restarted read syscall.
I don't quite follow what the goal of this test is. With orig_a0 being
added to the previous patch for ptrace, a more constrained test could
ensure that this value is respected.
Sry, I may not have described the patch clearly enough. This patch provides
a channel for modifying a0 in user-space ptrace via orig_a0. Here, I will
try to outline the whole situation:
1、When the tracer calls ptrace to modify regs->a0, can the tracee's a0 be
correctly modified?
Through testing, if the user only modifies regs->a0, it doesn't work. Why?
The execution flow of the tracee in the test program is as follows.Prior to
this explanation:
- PTRACE_SYSCALL can make the tracee block before and after executing
a syscall.
- The tracer sends SIGUSR1 to interrupt read, and the kernel will
restart it.
- Please note the point marked with (*), which I believe is the cause
of the issue.
user kernel
|
|
|
read
| +-> regs->orig_a0 = regs->a0; //(*1)
| <=tracer:PTRACE_SYSCALL
| +-> syscall_enter_from_user_mode
+-> ptrace_report_syscall_entry
+-> ptrace_stop
| //stopped
| <= tracer:SIGUSR1
|
| //resume <= tracer:PTRACE_SYSCALL
| syscall_handler...
|
| +-> syscall_exit_to_user_mode
+-> syscall_exit_to_user_mode_prepare
+-> ptrace_report_syscall_exit
+-> ptrace_stop
| //stopped
|
| /* Change a0/orig_a0 here and observe the restarted syscall */
| regs->{a0/orig_a0} = fd_zero; //(*2)
| ptrace(PTRACE_SETREGSET, ...);
| <= tracer:PTRACE_SYSCALL
| //restarting..., skip SIGUSR1
|
| +-> exit_to_user_mode_loop
+-> arch_do_signal_or_restart
+-> /* Settings for syscall restart */
regs->a0 = regs->orig_a0; //(*3)
| //stopped
| //and block before the syscall again, get current regs->a0
| *result = regs->a0;
|
| /* Now, Check regs->a0 of restarted syscall */
| EXPECT_NE(0x5, result); //for PTRACE_SETREGSSET a0, failed
| EXPECT_EQ(0x5, result); //for PTRACE_SETREGSSET orig_a0, succeed
If I'm wrong, please let me know. 🙂
2、Actually, I discovered the issue while using the execve function.
When I tried to modify the first parameter of execve in the tracer,
I found it didn't work.
As for why not use execve for testing, there are two reasons:
1) The root cause of this issue is that when a syscall is interrupted
and then resumed, it restarts with orig_a0 instead of a0, so modifying
a0 doesn't work. I want to focus the test on the "restarted syscall".
2) Compared to the current test scenario, execve is terminated by ptrace
earlier, so I chose a later point. In fact, setting regs->a0 in the path
between (*1) and (*3) is ineffective because it will eventually be
overwritten by orig_a0, correct?
Thank you for the thorough explanation! I feel like a test case like the
following achieves the same goal but without needing the pipes and the
file. What do you think?
From 5f13cdf8f7312b2b3cc98fbfbb3c95fcef62c0f0 Mon Sep 17 00:00:00 2001
From: Charlie Jenkins <charlie@xxxxxxxxxxxx>
Date: Fri, 21 Jun 2024 12:58:29 -0700
Subject: [PATCH] riscv: selftests: Add a ptrace test to check a0 of restarted
syscall
Add a riscv selftest that checks that a0 of syscalls are able to be
replaced. When entering a syscall, a0 contains the first argument to the
syscall and upon exiting, a0 contains the return value. To replace the
a0 argument instead of the a0 return value with ptrace after halting the
program with PTRACE_SYSCALL, orig_a0 must be used. This test checks that
orig_a0 allows a syscall argument to be modified, and that changing a0
does not change the syscall argument.
Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
---
.../riscv/abi/ptrace_restart_syscall.c | 123 ++++++++++++++++++
1 file changed, 123 insertions(+)
create mode 100644 tools/testing/selftests/riscv/abi/ptrace_restart_syscall.c
diff --git a/tools/testing/selftests/riscv/abi/ptrace_restart_syscall.c b/tools/testing/selftests/riscv/abi/ptrace_restart_syscall.c
new file mode 100644
index 000000000000..e74ae02c6850
--- /dev/null
+++ b/tools/testing/selftests/riscv/abi/ptrace_restart_syscall.c
@@ -0,0 +1,123 @@
+// 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 DEFAULT_A0 0x6
+
+#define ORIG_A0_AFTER_MODIFIED 0x5
+#define MODIFY_A0 0x01
+#define MODIFY_ORIG_A0 0x02
+
+#define perr_and_exit(fmt, ...) do { \
+ char buf[256]; \
+ snprintf(buf, sizeof(buf), "%s:%d: " fmt ": %m\n", \
+ __func__, __LINE__, ##__VA_ARGS__); \
+ perror(buf); \
+ exit(-1); \
+} while (0)
+
+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_restart_syscall(int opt, int *result)
+{
+ int status;
+ pid_t pid;
+
+ struct user_regs_struct regs;
+ struct iovec iov = {
+ .iov_base = ®s,
+ .iov_len = sizeof(regs),
+ };
+
+ 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(DEFAULT_A0);
+ } else 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 restarted syscall */
+ resume_and_wait_tracee(pid, PTRACE_SYSCALL);
+
+ /* Now, check regs.a0 of the restarted syscall */
+ if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))
+ perr_and_exit("failed to get tracee registers\n");
+
+ /* Modify a0/orig_a0 for the restarted syscall */
+ switch (opt) {
+ case MODIFY_A0:
+ regs.a0 = ORIG_A0_AFTER_MODIFIED;
+ break;
+ case MODIFY_ORIG_A0:
+ regs.orig_a0 = ORIG_A0_AFTER_MODIFIED;
+ break;
+ }
+
+ if (ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &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_restart_syscall(MODIFY_A0, &result);
+
+ /* The tracer's modification of a0 cannot affect the restarted tracee */
+ EXPECT_EQ(DEFAULT_A0, result);
+}
+
+TEST(ptrace_modify_orig_a0)
+{
+ int result;
+
+ ptrace_restart_syscall(MODIFY_ORIG_A0, &result);
+
+ /* The tracer must modify orig_a0 to actually change the tracee's a0 */
+ EXPECT_EQ(ORIG_A0_AFTER_MODIFIED, result);
+}
+
+TEST_HARNESS_MAIN
