Re: [RFC PATCH 2/2] riscv: selftests: Add a ptrace test to check a0 of restarted syscall
From: Charlie Jenkins
Date: Fri Jun 21 2024 - 16:20:28 EST
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?