[PATCH] arm64: Better native ptrace support for compat tasks

[PATCH] arm64: Better native ptrace support for compat tasks

[Catalin Marinas]

The compat ptrace interface allows access to the TLS register, hardware
breakpoints and watchpoints, syscall number. However, a native task
using the native ptrace interface to debug compat tasks (e.g. multi-arch
gdb) only has access to the general and VFP register sets. The compat
ptrace interface cannot be accessed from a native task.

This patch adds a new user_aarch32_ptrace_view which contains the TLS,
hardware breakpoint/watchpoint and syscall number regsets in addition to
the existing GPR and VFP regsets. This view is backwards compatible with
the previous kernels. Core dumping of 32-bit tasks and compat ptrace are
not affected since the original user_aarch32_view is preserved.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Yao Qi <yao.qi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/ptrace.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index d882b833dbdb..1971f491bb90 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -826,6 +826,30 @@ static int compat_vfp_set(struct task_struct *target,
 	return ret;
 }
 
+static int compat_tls_get(struct task_struct *target,
+			  const struct user_regset *regset, unsigned int pos,
+			  unsigned int count, void *kbuf, void __user *ubuf)
+{
+	compat_ulong_t tls = (compat_ulong_t)target->thread.tp_value;
+	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, &tls, 0, -1);
+}
+
+static int compat_tls_set(struct task_struct *target,
+			  const struct user_regset *regset, unsigned int pos,
+			  unsigned int count, const void *kbuf,
+			  const void __user *ubuf)
+{
+	int ret;
+	compat_ulong_t tls;
+
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &tls, 0, -1);
+	if (ret)
+		return ret;
+
+	target->thread.tp_value = tls;
+	return ret;
+}
+
 static const struct user_regset aarch32_regsets[] = {
 	[REGSET_COMPAT_GPR] = {
 		.core_note_type = NT_PRSTATUS,
@@ -850,6 +874,64 @@ static const struct user_regset_view user_aarch32_view = {
 	.regsets = aarch32_regsets, .n = ARRAY_SIZE(aarch32_regsets)
 };
 
+static const struct user_regset aarch32_ptrace_regsets[] = {
+	[REGSET_GPR] = {
+		.core_note_type = NT_PRSTATUS,
+		.n = COMPAT_ELF_NGREG,
+		.size = sizeof(compat_elf_greg_t),
+		.align = sizeof(compat_elf_greg_t),
+		.get = compat_gpr_get,
+		.set = compat_gpr_set
+	},
+	[REGSET_FPR] = {
+		.core_note_type = NT_ARM_VFP,
+		.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
+		.size = sizeof(compat_ulong_t),
+		.align = sizeof(compat_ulong_t),
+		.get = compat_vfp_get,
+		.set = compat_vfp_set
+	},
+	[REGSET_TLS] = {
+		.core_note_type = NT_ARM_TLS,
+		.n = 1,
+		.size = sizeof(compat_ulong_t),
+		.align = sizeof(compat_ulong_t),
+		.get = compat_tls_get,
+		.set = compat_tls_set,
+	},
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	[REGSET_HW_BREAK] = {
+		.core_note_type = NT_ARM_HW_BREAK,
+		.n = sizeof(struct user_hwdebug_state) / sizeof(u32),
+		.size = sizeof(u32),
+		.align = sizeof(u32),
+		.get = hw_break_get,
+		.set = hw_break_set,
+	},
+	[REGSET_HW_WATCH] = {
+		.core_note_type = NT_ARM_HW_WATCH,
+		.n = sizeof(struct user_hwdebug_state) / sizeof(u32),
+		.size = sizeof(u32),
+		.align = sizeof(u32),
+		.get = hw_break_get,
+		.set = hw_break_set,
+	},
+#endif
+	[REGSET_SYSTEM_CALL] = {
+		.core_note_type = NT_ARM_SYSTEM_CALL,
+		.n = 1,
+		.size = sizeof(int),
+		.align = sizeof(int),
+		.get = system_call_get,
+		.set = system_call_set,
+	},
+};
+
+static const struct user_regset_view user_aarch32_ptrace_view = {
+	.name = "aarch32", .e_machine = EM_ARM,
+	.regsets = aarch32_ptrace_regsets, .n = ARRAY_SIZE(aarch32_ptrace_regsets)
+};
+
 static int compat_ptrace_read_user(struct task_struct *tsk, compat_ulong_t off,
 				   compat_ulong_t __user *ret)
 {
@@ -1109,8 +1191,16 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 const struct user_regset_view *task_user_regset_view(struct task_struct *task)
 {
 #ifdef CONFIG_COMPAT
-	if (is_compat_thread(task_thread_info(task)))
+	/*
+	 * Core dumping of 32-bit tasks or compat ptrace requests must use the
+	 * user_aarch32_view compatible with arm32. Native ptrace requests on
+	 * 32-bit children use an extended user_aarch32_ptrace_view to allow
+	 * access to the TLS register.
+	 */
+	if (is_compat_task())
 		return &user_aarch32_view;
+	else if (is_compat_thread(task_thread_info(task)))
+		return &user_aarch32_ptrace_view;
 #endif
 	return &user_aarch64_view;
 }

[PATCH] arm64: Better native ptrace support for compat tasks

[Will Deacon]

Hi Catalin,

On Tue, Jul 14, 2015 at 04:20:17PM +0100, Catalin Marinas wrote:
> The compat ptrace interface allows access to the TLS register, hardware
> breakpoints and watchpoints, syscall number. However, a native task
> using the native ptrace interface to debug compat tasks (e.g. multi-arch
> gdb) only has access to the general and VFP register sets. The compat
> ptrace interface cannot be accessed from a native task.
> 
> This patch adds a new user_aarch32_ptrace_view which contains the TLS,
> hardware breakpoint/watchpoint and syscall number regsets in addition to
> the existing GPR and VFP regsets. This view is backwards compatible with
> the previous kernels. Core dumping of 32-bit tasks and compat ptrace are
> not affected since the original user_aarch32_view is preserved.

[...]

> +static const struct user_regset aarch32_ptrace_regsets[] = {
> +	[REGSET_GPR] = {
> +		.core_note_type = NT_PRSTATUS,
> +		.n = COMPAT_ELF_NGREG,
> +		.size = sizeof(compat_elf_greg_t),
> +		.align = sizeof(compat_elf_greg_t),
> +		.get = compat_gpr_get,
> +		.set = compat_gpr_set
> +	},
> +	[REGSET_FPR] = {
> +		.core_note_type = NT_ARM_VFP,
> +		.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
> +		.size = sizeof(compat_ulong_t),
> +		.align = sizeof(compat_ulong_t),
> +		.get = compat_vfp_get,
> +		.set = compat_vfp_set


I don't see how this is "backwards compatible with the previous kernels".
If there is some userspace out there that expects a native view onto the
registers of a compat task, then that's no longer the case, right? For
example, NT_PRFPREG requests will no longer work afaict. I'm not sure if
it matters that the NT_PRSTATUS regset is now limited to COMPAT_ELF_NGREG.

Will

[PATCH] arm64: Better native ptrace support for compat tasks

[Catalin Marinas]

Hi Will,

On Fri, Jul 17, 2015 at 11:26:29AM +0100, Will Deacon wrote:
> On Tue, Jul 14, 2015 at 04:20:17PM +0100, Catalin Marinas wrote:
> > The compat ptrace interface allows access to the TLS register, hardware
> > breakpoints and watchpoints, syscall number. However, a native task
> > using the native ptrace interface to debug compat tasks (e.g. multi-arch
> > gdb) only has access to the general and VFP register sets. The compat
> > ptrace interface cannot be accessed from a native task.
> > 
> > This patch adds a new user_aarch32_ptrace_view which contains the TLS,
> > hardware breakpoint/watchpoint and syscall number regsets in addition to
> > the existing GPR and VFP regsets. This view is backwards compatible with
> > the previous kernels. Core dumping of 32-bit tasks and compat ptrace are
> > not affected since the original user_aarch32_view is preserved.
> 
> [...]
> 
> > +static const struct user_regset aarch32_ptrace_regsets[] = {
> > +	[REGSET_GPR] = {
> > +		.core_note_type = NT_PRSTATUS,
> > +		.n = COMPAT_ELF_NGREG,
> > +		.size = sizeof(compat_elf_greg_t),
> > +		.align = sizeof(compat_elf_greg_t),
> > +		.get = compat_gpr_get,
> > +		.set = compat_gpr_set
> > +	},
> > +	[REGSET_FPR] = {
> > +		.core_note_type = NT_ARM_VFP,
> > +		.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
> > +		.size = sizeof(compat_ulong_t),
> > +		.align = sizeof(compat_ulong_t),
> > +		.get = compat_vfp_get,
> > +		.set = compat_vfp_set
> 
> 
> I don't see how this is "backwards compatible with the previous kernels".

The first two entries of aarch32_ptrace_regsets are the same as the two
aarch32_regsets entries currently in use before this patch (I just
dropped "COMPAT" from REGSET_* for consistency with the rest of the
array).

> If there is some userspace out there that expects a native view onto the
> registers of a compat task, then that's no longer the case, right? For
> example, NT_PRFPREG requests will no longer work afaict.

But this never worked since task_user_regset_view() always returned
user_aarch32_view (with the aarch32_regsets) if the ptrace'd task was
compat. So a native gdb debugging compat task is expected to use
NT_ARM_VFP before and after this patch.

I don't think it's worth adding another regset for NT_PRFPREG just to
access the 128-bit FP registers since the compat task doesn't see them
anyway. It's the same reason NT_PRSTATUS above would not return 64-bit
registers.

> I'm not sure if it matters that the NT_PRSTATUS regset is now limited
> to COMPAT_ELF_NGREG.

Already commented above. I don't really think it matters, nor can we do
it without breaking the ABI. And it also makes you think a bit more
about security (though we should be safe as we sanitise the top part of
the 64-bit registers).

-- 
Catalin

[PATCH] arm64: Better native ptrace support for compat tasks

[Will Deacon]

On Fri, Jul 17, 2015 at 11:49:54AM +0100, Catalin Marinas wrote:
> On Fri, Jul 17, 2015 at 11:26:29AM +0100, Will Deacon wrote:
> > On Tue, Jul 14, 2015 at 04:20:17PM +0100, Catalin Marinas wrote:
> > > The compat ptrace interface allows access to the TLS register, hardware
> > > breakpoints and watchpoints, syscall number. However, a native task
> > > using the native ptrace interface to debug compat tasks (e.g. multi-arch
> > > gdb) only has access to the general and VFP register sets. The compat
> > > ptrace interface cannot be accessed from a native task.
> > > 
> > > This patch adds a new user_aarch32_ptrace_view which contains the TLS,
> > > hardware breakpoint/watchpoint and syscall number regsets in addition to
> > > the existing GPR and VFP regsets. This view is backwards compatible with
> > > the previous kernels. Core dumping of 32-bit tasks and compat ptrace are
> > > not affected since the original user_aarch32_view is preserved.
> > 
> > [...]
> > 
> > > +static const struct user_regset aarch32_ptrace_regsets[] = {
> > > +	[REGSET_GPR] = {
> > > +		.core_note_type = NT_PRSTATUS,
> > > +		.n = COMPAT_ELF_NGREG,
> > > +		.size = sizeof(compat_elf_greg_t),
> > > +		.align = sizeof(compat_elf_greg_t),
> > > +		.get = compat_gpr_get,
> > > +		.set = compat_gpr_set
> > > +	},
> > > +	[REGSET_FPR] = {
> > > +		.core_note_type = NT_ARM_VFP,
> > > +		.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
> > > +		.size = sizeof(compat_ulong_t),
> > > +		.align = sizeof(compat_ulong_t),
> > > +		.get = compat_vfp_get,
> > > +		.set = compat_vfp_set
> > 
> > 
> > I don't see how this is "backwards compatible with the previous kernels".
> 
> The first two entries of aarch32_ptrace_regsets are the same as the two
> aarch32_regsets entries currently in use before this patch (I just
> dropped "COMPAT" from REGSET_* for consistency with the rest of the
> array).
> 
> > If there is some userspace out there that expects a native view onto the
> > registers of a compat task, then that's no longer the case, right? For
> > example, NT_PRFPREG requests will no longer work afaict.
> 
> But this never worked since task_user_regset_view() always returned
> user_aarch32_view (with the aarch32_regsets) if the ptrace'd task was
> compat. So a native gdb debugging compat task is expected to use
> NT_ARM_VFP before and after this patch.

Aha, sorry, I got confused because you've wired up the native hw-breakpoint
interfaces later on in the regset (which is fine, because I don't think
the parent could've got into the compat hw-breakpoint ptrace requests
since they're not regset-based).

So I think this looks fine, thanks!

Will