| Message ID | d4c7da80b72375c75836303bc744e4db9eeec218.1718693532.git.zhouquan@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Expose orig_a0 to userspace for ptrace to set the actual a0 | expand |
On Wed, Jun 19, 2024 at 10:01:27AM +0800, zhouquan@iscas.ac.cn wrote: > From: Quan Zhou <zhouquan@iscas.ac.cn> > > Expose orig_a0 to userspace to ensure that users can modify > the actual value of `a0` in the traced process through the > ptrace(PTRACE_SETREGSET, ...) path. Since user_regs_struct is > a subset of pt_regs, we also need to adjust the position of > the orig_a0 field in pt_regs to achieve the correct copy. > > Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn> > --- > arch/riscv/include/asm/ptrace.h | 4 ++-- > arch/riscv/include/uapi/asm/ptrace.h | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h > index b5b0adcc85c1..37f48d40ae46 100644 > --- a/arch/riscv/include/asm/ptrace.h > +++ b/arch/riscv/include/asm/ptrace.h > @@ -45,12 +45,12 @@ struct pt_regs { > unsigned long t4; > unsigned long t5; > unsigned long t6; > + /* a0 value before the syscall */ > + unsigned long orig_a0; > /* Supervisor/Machine CSRs */ > unsigned long status; > unsigned long badaddr; > unsigned long cause; > - /* a0 value before the syscall */ > - unsigned long orig_a0; > }; > > #define PTRACE_SYSEMU 0x1f > diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h > index a38268b19c3d..3e37f80cb3e8 100644 > --- a/arch/riscv/include/uapi/asm/ptrace.h > +++ b/arch/riscv/include/uapi/asm/ptrace.h > @@ -54,6 +54,8 @@ struct user_regs_struct { > unsigned long t4; > unsigned long t5; > unsigned long t6; > + /* a0 value before the syscall */ > + unsigned long orig_a0; > }; > > struct __riscv_f_ext_state { > -- > 2.34.1 > This is a good addition! Since orig_a0 is no longer at the bottom of pt_regs, MAX_REG_OFFSET is now incorrect. Can you adjust the value of: #define MAX_REG_OFFSET offsetof(struct pt_regs, orig_a0) in arch/riscv/include/asm/ptrace.h to be: #define MAX_REG_OFFSET offsetof(struct pt_regs, cause) This is something that is very easy to miss. I think it would be valuable to leave a comment at the top of struct pt_regs pointing out that MAX_REG_OFFSET needs to be adjusted if struct pt_regs changes. - Charlie
On 2024/6/20 09:05, Charlie Jenkins wrote: > On Wed, Jun 19, 2024 at 10:01:27AM +0800, zhouquan@iscas.ac.cn wrote: >> From: Quan Zhou <zhouquan@iscas.ac.cn> >> >> Expose orig_a0 to userspace to ensure that users can modify >> the actual value of `a0` in the traced process through the >> ptrace(PTRACE_SETREGSET, ...) path. Since user_regs_struct is >> a subset of pt_regs, we also need to adjust the position of >> the orig_a0 field in pt_regs to achieve the correct copy. >> >> Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn> >> --- >> arch/riscv/include/asm/ptrace.h | 4 ++-- >> arch/riscv/include/uapi/asm/ptrace.h | 2 ++ >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h >> index b5b0adcc85c1..37f48d40ae46 100644 >> --- a/arch/riscv/include/asm/ptrace.h >> +++ b/arch/riscv/include/asm/ptrace.h >> @@ -45,12 +45,12 @@ struct pt_regs { >> unsigned long t4; >> unsigned long t5; >> unsigned long t6; >> + /* a0 value before the syscall */ >> + unsigned long orig_a0; >> /* Supervisor/Machine CSRs */ >> unsigned long status; >> unsigned long badaddr; >> unsigned long cause; >> - /* a0 value before the syscall */ >> - unsigned long orig_a0; >> }; >> >> #define PTRACE_SYSEMU 0x1f >> diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h >> index a38268b19c3d..3e37f80cb3e8 100644 >> --- a/arch/riscv/include/uapi/asm/ptrace.h >> +++ b/arch/riscv/include/uapi/asm/ptrace.h >> @@ -54,6 +54,8 @@ struct user_regs_struct { >> unsigned long t4; >> unsigned long t5; >> unsigned long t6; >> + /* a0 value before the syscall */ >> + unsigned long orig_a0; >> }; >> >> struct __riscv_f_ext_state { >> -- >> 2.34.1 >> > This is a good addition! > > Since orig_a0 is no longer at the bottom of pt_regs, MAX_REG_OFFSET is > now incorrect. > > Can you adjust the value of: > > #define MAX_REG_OFFSET offsetof(struct pt_regs, orig_a0) > > in arch/riscv/include/asm/ptrace.h to be: > > #define MAX_REG_OFFSET offsetof(struct pt_regs, cause) > > This is something that is very easy to miss. I think it would be > valuable to leave a comment at the top of struct pt_regs pointing out > that MAX_REG_OFFSET needs to be adjusted if struct pt_regs changes. > > - Charlie > Oh, good idea! I'm sorry for missing such an important point earlier. Once enough information is gathered from this RFC, I will include what you mentioned in the V1 series. Thanks, Quan > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h index b5b0adcc85c1..37f48d40ae46 100644 --- a/arch/riscv/include/asm/ptrace.h +++ b/arch/riscv/include/asm/ptrace.h @@ -45,12 +45,12 @@ struct pt_regs { unsigned long t4; unsigned long t5; unsigned long t6; + /* a0 value before the syscall */ + unsigned long orig_a0; /* Supervisor/Machine CSRs */ unsigned long status; unsigned long badaddr; unsigned long cause; - /* a0 value before the syscall */ - unsigned long orig_a0; }; #define PTRACE_SYSEMU 0x1f diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h index a38268b19c3d..3e37f80cb3e8 100644 --- a/arch/riscv/include/uapi/asm/ptrace.h +++ b/arch/riscv/include/uapi/asm/ptrace.h @@ -54,6 +54,8 @@ struct user_regs_struct { unsigned long t4; unsigned long t5; unsigned long t6; + /* a0 value before the syscall */ + unsigned long orig_a0; }; struct __riscv_f_ext_state {