[PATCH -next 0/2] riscv: ptrace: implement PTRACE_{PEEK,POKE}USR

[PATCH -next 0/2] riscv: ptrace: implement PTRACE_{PEEK,POKE}USR

[Yipeng Zou]

The PTRACE_{PEEK,POKE}USR going to get/set tracee's user register.
This patch sets add PTRACE_{PEEK,POKE}USR and compat implement to
riscv.

I notice this issuse when transplanting some app form x86 to riscv
recently.
I don't know why these two function are not implemented now, and I
found few talk about that.

The {PEEK,POKE}USR are both defined in include\uapi\linux\ptrace.h,
So I was thinking maybe these two function shouled be implemented.

Yipeng Zou (2):
  riscv: ptrace: Implement PTRACE_{PEEK,POKE}USR
  riscv: ptrace: Implement Compat PTRACE_{PEEK,POKE}USR

 arch/riscv/include/asm/compat.h |  13 ++++
 arch/riscv/kernel/ptrace.c      | 111 ++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH -next 1/2] riscv: ptrace: Implement PTRACE_{PEEK,POKE}USR

[Yipeng Zou]

Implement PTRACE_{PEEK,POKE}USR for RV64 user app to peek/poke tracee's
space register.

As the Linux manual page say: the addr is define to a word offset in
the tracee's USER area.In riscv it's defined in:

struct user {
	struct user_regs {
		ulong pc;	// addr 0
		ulong ra;	// addr 8
		...
		...
		ulong t6;	// addr 248
	}
	struct d_ext_state {
		u64 f[0];	// addr 256
		...
		...
		u64 f[31];	// addr 504
		u32 fcsr;	// addr 512 read-only
	}
}

Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
---
 arch/riscv/kernel/ptrace.c | 66 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 2ae8280ae475..6dc0687e419f 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -215,12 +215,78 @@ void ptrace_disable(struct task_struct *child)
 	clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
 }
 
+static inline unsigned long ptrace_get_gpr_reg(struct task_struct *child, unsigned long offset)
+{
+	struct pt_regs *regs = task_pt_regs(child);
+
+	return *((unsigned long *)regs + offset / sizeof(regs->a0));
+}
+
+static inline int ptrace_set_gpr_reg(struct task_struct *child, unsigned long offset,
+				     unsigned long data)
+{
+	struct pt_regs *regs = task_pt_regs(child);
+
+	*((unsigned long *)regs + offset / sizeof(regs->a0)) = data;
+	return 0;
+}
+
+#ifdef CONFIG_FPU
+static inline unsigned long ptrace_get_fpr_reg(struct task_struct *child, unsigned long offset)
+{
+	struct __riscv_d_ext_state *fstate = &child->thread.fstate;
+
+	if (offset <= offsetof(struct __riscv_d_ext_state, f[31]))
+		return fstate->f[offset / sizeof(fstate->f[0])];
+	else if (offset == offsetof(struct __riscv_d_ext_state, fcsr))
+		return fstate->fcsr;
+	return -EIO;
+}
+
+static inline int ptrace_set_fpr_reg(struct task_struct *child, unsigned long offset,
+				     unsigned long data)
+{
+	struct __riscv_d_ext_state *fstate = &child->thread.fstate;
+
+	if (offset <= offsetof(struct __riscv_d_ext_state, f[31]))
+		fstate->f[offset / sizeof(fstate->f[0])] = data;
+	else /* Only f[0] ~ f[31] can write by PTRACE_POKEUSR */
+		return -EIO;
+	return 0;
+}
+#endif
+
 long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	long ret = -EIO;
+	unsigned long __user *datap = (unsigned long __user *)data;
 
 	switch (request) {
+	case PTRACE_PEEKUSR:
+	case PTRACE_POKEUSR:
+		/* addr must be aligned to register size */
+		if (addr & (sizeof(addr) - 1))
+			break;
+
+		if (addr < sizeof(struct user_regs_struct)) {
+			if (request == PTRACE_PEEKUSR)
+				ret = put_user(ptrace_get_gpr_reg(child, addr), datap);
+			else	// PTRACE_POKEUSR
+				ret = ptrace_set_gpr_reg(child, addr, data);
+#ifdef CONFIG_FPU
+		} else if (addr < (sizeof(struct user_regs_struct) +
+				   sizeof(struct __riscv_d_ext_state))) {
+			addr -= sizeof(struct user_regs_struct);
+			if (request == PTRACE_PEEKUSR)
+				ret = put_user(ptrace_get_fpr_reg(child, addr), datap);
+			else	// PTRACE_POKEUSR
+				ret = ptrace_set_fpr_reg(child, addr, data);
+#endif
+		} else {
+			// addr invalid
+		}
+		break;
 	default:
 		ret = ptrace_request(child, request, addr, data);
 		break;
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH -next 2/2] riscv: ptrace: Implement Compat PTRACE_{PEEK,POKE}USR

[Yipeng Zou]

Implement Compat PTRACE_{PEEK,POKE}USR for RV32 user app to peek/poke
tracee's space register.

As the Linux manual page say: the addr is define to a word offset in
the tracee's USER area, In riscv it's defined in:

struct user {
	struct user_regs {
		compat_ulong pc;	// addr 0
		compat_ulong ra;	// addr 4
		...
		...
		compat_ulong t6;	// addr 124
	}
	struct d_ext_state {
		u64 f[0]		// addr 128
		u64 f[1]		// addr 136
		...
		...
		u32 fcsr;		// addr 384 read-only
	}
}

Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
---
 arch/riscv/include/asm/compat.h | 13 ++++++++++
 arch/riscv/kernel/ptrace.c      | 45 +++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/arch/riscv/include/asm/compat.h b/arch/riscv/include/asm/compat.h
index 2ac955b51148..a6fc3706a26d 100644
--- a/arch/riscv/include/asm/compat.h
+++ b/arch/riscv/include/asm/compat.h
@@ -52,6 +52,19 @@ struct compat_user_regs_struct {
 	compat_ulong_t t6;
 };
 
+static inline compat_ulong_t creg_get_by_offset(struct pt_regs *regs, compat_ulong_t offset)
+{
+	unsigned long reg_value = *((unsigned long *)regs + offset / sizeof(compat_ulong_t));
+
+	return (compat_ulong_t)reg_value;
+}
+
+static inline void regs_set_by_offset(struct pt_regs *regs, compat_ulong_t offset,
+				      compat_ulong_t data)
+{
+	*((unsigned long *)regs + offset / sizeof(compat_ulong_t)) = (unsigned long)data;
+}
+
 static inline void regs_to_cregs(struct compat_user_regs_struct *cregs,
 				 struct pt_regs *regs)
 {
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 6dc0687e419f..22fedc13cbd2 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -391,12 +391,57 @@ static const struct user_regset_view compat_riscv_user_native_view = {
 	.n = ARRAY_SIZE(compat_riscv_user_regset),
 };
 
+static inline unsigned long compat_ptrace_get_gpr_reg(struct task_struct *child,
+						      compat_ulong_t offset)
+{
+	return creg_get_by_offset(task_pt_regs(child), offset);
+}
+
+static inline int compat_ptrace_set_gpr_reg(struct task_struct *child, compat_ulong_t offset,
+					    compat_ulong_t data)
+{
+	regs_set_by_offset(task_pt_regs(child), offset, data);
+	return 0;
+}
+
 long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 			compat_ulong_t caddr, compat_ulong_t cdata)
 {
 	long ret = -EIO;
+	unsigned long data = cdata;
+	compat_ulong_t __user *cdatap = (compat_ulong_t __user *)compat_ptr(data);
 
 	switch (request) {
+	case PTRACE_PEEKUSR:
+	case PTRACE_POKEUSR:
+		if (caddr < sizeof(struct compat_user_regs_struct)) {
+			/* access gprs addr must be aligned to compat_ulong_t */
+			if (caddr & (sizeof(caddr) - 1))
+				break;
+
+			if (request == PTRACE_PEEKUSR)
+				ret = put_user(compat_ptrace_get_gpr_reg(child, caddr), cdatap);
+			else	// PTRACE_POKEUSR
+				ret = compat_ptrace_set_gpr_reg(child, caddr, cdata);
+#ifdef CONFIG_FPU
+		} else if (caddr < (sizeof(struct compat_user_regs_struct) +
+				    sizeof(struct __riscv_d_ext_state))) {
+			caddr -= sizeof(struct compat_user_regs_struct);
+			/* access fprs addr must be aligned to __u64 */
+			if (caddr & (sizeof(__u64) - 1))
+				break;
+
+			if (request == PTRACE_PEEKUSR)
+				ret = put_user(ptrace_get_fpr_reg(child, (unsigned long)caddr),
+					       cdatap);
+			else	// PTRACE_POKEUSR
+				ret = ptrace_set_fpr_reg(child, (unsigned long)caddr,
+							 (unsigned long)cdata);
+#endif
+		} else {
+			// caddr invalid
+		}
+		break;
 	default:
 		ret = compat_ptrace_request(child, request, caddr, cdata);
 		break;
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH -next 0/2] riscv: ptrace: implement PTRACE_{PEEK,POKE}USR

[Andreas Schwab]

On Aug 22 2022, Yipeng Zou wrote:

> The PTRACE_{PEEK,POKE}USR going to get/set tracee's user register.
> This patch sets add PTRACE_{PEEK,POKE}USR and compat implement to
> riscv.
>
> I notice this issuse when transplanting some app form x86 to riscv
> recently.
> I don't know why these two function are not implemented now, and I
> found few talk about that.
>
> The {PEEK,POKE}USR are both defined in include\uapi\linux\ptrace.h,
> So I was thinking maybe these two function shouled be implemented.

There are PTRACE_[GS]ETREGSET.  I think PTRACE_{PEEK,POKE}USR are
legacy-only.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH -next 0/2] riscv: ptrace: implement PTRACE_{PEEK,POKE}USR

[Palmer Dabbelt]

On Mon, 22 Aug 2022 01:19:03 PDT (-0700), schwab@suse.de wrote:
> On Aug 22 2022, Yipeng Zou wrote:
>
>> The PTRACE_{PEEK,POKE}USR going to get/set tracee's user register.
>> This patch sets add PTRACE_{PEEK,POKE}USR and compat implement to
>> riscv.
>>
>> I notice this issuse when transplanting some app form x86 to riscv
>> recently.
>> I don't know why these two function are not implemented now, and I
>> found few talk about that.
>>
>> The {PEEK,POKE}USR are both defined in include\uapi\linux\ptrace.h,
>> So I was thinking maybe these two function shouled be implemented.
>
> There are PTRACE_[GS]ETREGSET.  I think PTRACE_{PEEK,POKE}USR are
> legacy-only.

That's my understanding as well.  Anything ported to RISC-V would need 
to understand the new register formats, so there's not much benefit to 
supporting these old flavors of register twiddling -- maybe some 
refactoring needs to be done in userspace to take advantage of accessing 
multiple registers at the same time, but that should probably be done 
anyway as all ports support the new interfaces these days.

There's been a few rounds of these having been posted, maybe that means 
the documentation should be improved?  I sent 
<https://lore.kernel.org/r/20221006012432.19008-1-palmer@rivosinc.com/>, 
that'll also help sort out whether we're supposed to have these.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH -next 0/2] riscv: ptrace: implement PTRACE_{PEEK,POKE}USR

[Yipeng Zou]

在 2022/10/6 9:26, Palmer Dabbelt 写道:
> On Mon, 22 Aug 2022 01:19:03 PDT (-0700), schwab@suse.de wrote:
>> On Aug 22 2022, Yipeng Zou wrote:
>>
>>> The PTRACE_{PEEK,POKE}USR going to get/set tracee's user register.
>>> This patch sets add PTRACE_{PEEK,POKE}USR and compat implement to
>>> riscv.
>>>
>>> I notice this issuse when transplanting some app form x86 to riscv
>>> recently.
>>> I don't know why these two function are not implemented now, and I
>>> found few talk about that.
>>>
>>> The {PEEK,POKE}USR are both defined in include\uapi\linux\ptrace.h,
>>> So I was thinking maybe these two function shouled be implemented.
>>
>> There are PTRACE_[GS]ETREGSET.  I think PTRACE_{PEEK,POKE}USR are
>> legacy-only.
>
> That's my understanding as well.  Anything ported to RISC-V would need 
> to understand the new register formats, so there's not much benefit to 
> supporting these old flavors of register twiddling -- maybe some 
> refactoring needs to be done in userspace to take advantage of 
> accessing multiple registers at the same time, but that should 
> probably be done anyway as all ports support the new interfaces these 
> days.
>
ok, got it.
> There's been a few rounds of these having been posted, maybe that 
> means the documentation should be improved?  I sent 
> <https://lore.kernel.org/r/20221006012432.19008-1-palmer@rivosinc.com/>, 
> that'll also help sort out whether we're supposed to have these.

-- 
Regards,
Yipeng Zou


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv