diff mbox series

[18/20] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

Message ID 20170604120009.342-19-ynorov@caviumnetworks.com
State Superseded
Headers show
Series [01/20] compat ABI: use non-compat openat and open_by_handle_at variants | expand

Commit Message

Yury Norov June 4, 2017, noon UTC 
ILP32 has context-related structures different from both aarch32 and
aarch64/lp64. In this patch compat_arch_ptrace() renamed to
compat_a32_ptrace(), and compat_arch_ptrace() only makes choice between
compat_a32_ptrace() and new compat_ilp32_ptrace() handler.

compat_ilp32_ptrace() calls generic compat_ptrace_request() for all
requests except PTRACE_GETSIGMASK and PTRACE_SETSIGMASK, which need
special handling.

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

Signed-off-by: Chengming Zhou <zhouchengming1@huawei.com>

---
 arch/arm64/kernel/ptrace.c | 65 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

-- 
2.11.0

Comments

James Morse June 23, 2017, 5:03 p.m. UTC | #1 
Hi Yury,

On 04/06/17 13:00, Yury Norov wrote:
> ILP32 has context-related structures different from both aarch32 and

> aarch64/lp64. In this patch compat_arch_ptrace() renamed to

> compat_a32_ptrace(), and compat_arch_ptrace() only makes choice between

> compat_a32_ptrace() and new compat_ilp32_ptrace() handler.

> 

> compat_ilp32_ptrace() calls generic compat_ptrace_request() for all

> requests except PTRACE_GETSIGMASK and PTRACE_SETSIGMASK, which need

> special handling.


Can you elaborate on this special handling?

How come we don't need to wrap PTRACE_{G,S}ETSIGMASK for aarch32 compat?
From kernel/signal32.c that uses compat_sigset_t too.

It looks like aarch64, ilp32 and aarch32 all use the same size sigset_t,
so doesn't compat_ptrace_request() already do everything we need?

...

Is this fixing an endian problem? If so, can we document it as such. Do we
already have the same bug for aarch32 compat?


Thanks,

James
Yury Norov June 23, 2017, 10:28 p.m. UTC | #2 
On Fri, Jun 23, 2017 at 06:03:37PM +0100, James Morse wrote:
> Hi Yury,

> 

> On 04/06/17 13:00, Yury Norov wrote:

> > ILP32 has context-related structures different from both aarch32 and

> > aarch64/lp64. In this patch compat_arch_ptrace() renamed to

> > compat_a32_ptrace(), and compat_arch_ptrace() only makes choice between

> > compat_a32_ptrace() and new compat_ilp32_ptrace() handler.

> > 

> > compat_ilp32_ptrace() calls generic compat_ptrace_request() for all

> > requests except PTRACE_GETSIGMASK and PTRACE_SETSIGMASK, which need

> > special handling.

> 

> Can you elaborate on this special handling?

> 

> How come we don't need to wrap PTRACE_{G,S}ETSIGMASK for aarch32 compat?

> >From kernel/signal32.c that uses compat_sigset_t too.

> 

> It looks like aarch64, ilp32 and aarch32 all use the same size sigset_t,

> so doesn't compat_ptrace_request() already do everything we need?

> 

> ...

> 

> Is this fixing an endian problem? If so, can we document it as such. Do we

> already have the same bug for aarch32 compat?


Originally, the problem was found by Zhou Chengming: https://lkml.org/lkml/2016/6/27/18
But I think you right, this is the fix for endian.

It lookd like aarch32 is buggy, but IIUC to confirm it, the BE arm64
machine is needed. I use qemu and AFAIR it has no BE support.

Zhou, can you test it on your machine and if the bug will be reproduced,
send the patch for aarch32?

Yury
James Morse June 27, 2017, 10:12 a.m. UTC | #3 
Hi Yury, Zhou,

On 23/06/17 23:28, Yury Norov wrote:
> On Fri, Jun 23, 2017 at 06:03:37PM +0100, James Morse wrote:

>> Hi Yury,

>>

>> On 04/06/17 13:00, Yury Norov wrote:

>>> ILP32 has context-related structures different from both aarch32 and

>>> aarch64/lp64. In this patch compat_arch_ptrace() renamed to

>>> compat_a32_ptrace(), and compat_arch_ptrace() only makes choice between

>>> compat_a32_ptrace() and new compat_ilp32_ptrace() handler.

>>>

>>> compat_ilp32_ptrace() calls generic compat_ptrace_request() for all

>>> requests except PTRACE_GETSIGMASK and PTRACE_SETSIGMASK, which need

>>> special handling.

>>

>> Can you elaborate on this special handling?

>>

>> How come we don't need to wrap PTRACE_{G,S}ETSIGMASK for aarch32 compat?

>> >From kernel/signal32.c that uses compat_sigset_t too.

>>

>> It looks like aarch64, ilp32 and aarch32 all use the same size sigset_t,

>> so doesn't compat_ptrace_request() already do everything we need?

>>

>> ...

>>

>> Is this fixing an endian problem? If so, can we document it as such. Do we

>> already have the same bug for aarch32 compat?

> 

> Originally, the problem was found by Zhou Chengming: https://lkml.org/lkml/2016/6/27/18

> But I think you right, this is the fix for endian.

> 

> It lookd like aarch32 is buggy, but IIUC to confirm it, the BE arm64

> machine is needed. I use qemu and AFAIR it has no BE support.

> 

> Zhou, can you test it on your machine and if the bug will be reproduced,

> send the patch for aarch32?


I've reproduced this on big endian compat-aarch32: yes its broken. I will respin
Zhou's patch as a fix.


Thanks,

James
diff mbox series

Patch

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index e2b7c040bf84..28f96765e8cc 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -765,9 +765,11 @@  static const struct user_regset_view user_aarch64_view = {
 	.regsets = aarch64_regsets, .n = ARRAY_SIZE(aarch64_regsets)
 };
 
-#ifdef CONFIG_AARCH32_EL0
+#ifdef CONFIG_COMPAT
 #include <linux/compat.h>
+#endif
 
+#ifdef CONFIG_AARCH32_EL0
 enum compat_regset {
 	REGSET_COMPAT_GPR,
 	REGSET_COMPAT_VFP,
@@ -1223,7 +1225,7 @@  static int compat_ptrace_sethbpregs(struct task_struct *tsk, compat_long_t num,
 }
 #endif	/* CONFIG_HAVE_HW_BREAKPOINT */
 
-long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+static long compat_a32_ptrace(struct task_struct *child, compat_long_t request,
 			compat_ulong_t caddr, compat_ulong_t cdata)
 {
 	unsigned long addr = caddr;
@@ -1300,8 +1302,67 @@  long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 
 	return ret;
 }
+
+#else
+#define  compat_a32_ptrace(child, request, caddr, cdata)	(0)
 #endif /* CONFIG_AARCH32_EL0 */
 
+#ifdef CONFIG_ARM64_ILP32
+#include <asm/signal32_common.h>
+
+static long compat_ilp32_ptrace(struct task_struct *child, compat_long_t request,
+			compat_ulong_t caddr, compat_ulong_t cdata)
+{
+	sigset_t new_set;
+
+	switch (request) {
+	case PTRACE_GETSIGMASK:
+		if (caddr != sizeof(compat_sigset_t))
+			return -EINVAL;
+
+		return put_sigset_t((compat_sigset_t __user *) (u64) cdata,
+					&child->blocked);
+
+	case PTRACE_SETSIGMASK:
+		if (caddr != sizeof(compat_sigset_t))
+			return -EINVAL;
+
+		if (get_sigset_t(&new_set, (compat_sigset_t __user *) (u64) cdata))
+			return -EFAULT;
+
+		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+		/*
+		 * Every thread does recalc_sigpending() after resume, so
+		 * retarget_shared_pending() and recalc_sigpending() are not
+		 * called here.
+		 */
+		spin_lock_irq(&child->sighand->siglock);
+		child->blocked = new_set;
+		spin_unlock_irq(&child->sighand->siglock);
+
+		return 0;
+
+	default:
+		return compat_ptrace_request(child, request, caddr, cdata);
+	}
+}
+
+#else
+#define compat_ilp32_ptrace(child, request, caddr, cdata)	(0)
+#endif
+
+#ifdef CONFIG_COMPAT
+long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+			compat_ulong_t caddr, compat_ulong_t cdata)
+{
+	if (is_a32_compat_task())
+		return compat_a32_ptrace(child, request, caddr, cdata);
+
+	return compat_ilp32_ptrace(child, request, caddr, cdata);
+}
+#endif
+
 const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 {
 #ifdef CONFIG_AARCH32_EL0