2021-01-19 22:15:59

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers

This is an alternative to NT_PRSTATUS that clobbers ip/r12 on AArch32,
x7 on AArch64 when a tracee is stopped in syscall entry or syscall exit
traps.

Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/kernel/ptrace.c | 39 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/elf.h | 1 +
2 files changed, 40 insertions(+)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 1863f080cb07..b8e4c2ddf636 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -591,6 +591,15 @@ static int gpr_get(struct task_struct *target,
return ret;
}

+static int gpr_get_full(struct task_struct *target,
+ const struct user_regset *regset,
+ struct membuf to)
+{
+ struct user_pt_regs *uregs = &task_pt_regs(target)->user_regs;
+
+ return membuf_write(&to, uregs, sizeof(*uregs));
+}
+
static int gpr_set(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
@@ -1088,6 +1097,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target, const struct

enum aarch64_regset {
REGSET_GPR,
+ REGSET_GPR_FULL,
REGSET_FPR,
REGSET_TLS,
#ifdef CONFIG_HAVE_HW_BREAKPOINT
@@ -1119,6 +1129,14 @@ static const struct user_regset aarch64_regsets[] = {
.regset_get = gpr_get,
.set = gpr_set
},
+ [REGSET_GPR_FULL] = {
+ .core_note_type = NT_ARM_PRSTATUS,
+ .n = sizeof(struct user_pt_regs) / sizeof(u64),
+ .size = sizeof(u64),
+ .align = sizeof(u64),
+ .regset_get = gpr_get_full,
+ .set = gpr_set
+ },
[REGSET_FPR] = {
.core_note_type = NT_PRFPREG,
.n = sizeof(struct user_fpsimd_state) / sizeof(u32),
@@ -1225,6 +1243,7 @@ static const struct user_regset_view user_aarch64_view = {
#ifdef CONFIG_COMPAT
enum compat_regset {
REGSET_COMPAT_GPR,
+ REGSET_COMPAT_GPR_FULL,
REGSET_COMPAT_VFP,
};

@@ -1285,6 +1304,18 @@ static int compat_gpr_get(struct task_struct *target,
return 0;
}

+/* compat_gpr_get_full doesn't overwrite x12 like compat_gpr_get. */
+static int compat_gpr_get_full(struct task_struct *target,
+ const struct user_regset *regset,
+ struct membuf to)
+{
+ int i = 0;
+
+ while (to.left)
+ membuf_store(&to, compat_get_user_reg(target, i++));
+ return 0;
+}
+
static int compat_gpr_set(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
@@ -1435,6 +1466,14 @@ static const struct user_regset aarch32_regsets[] = {
.regset_get = compat_gpr_get,
.set = compat_gpr_set
},
+ [REGSET_COMPAT_GPR_FULL] = {
+ .core_note_type = NT_ARM_PRSTATUS,
+ .n = COMPAT_ELF_NGREG,
+ .size = sizeof(compat_elf_greg_t),
+ .align = sizeof(compat_elf_greg_t),
+ .regset_get = compat_gpr_get_full,
+ .set = compat_gpr_set
+ },
[REGSET_COMPAT_VFP] = {
.core_note_type = NT_ARM_VFP,
.n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 30f68b42eeb5..a2086d19263a 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -426,6 +426,7 @@ typedef struct elf64_shdr {
#define NT_ARM_PACA_KEYS 0x407 /* ARM pointer authentication address keys */
#define NT_ARM_PACG_KEYS 0x408 /* ARM pointer authentication generic key */
#define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* arm64 tagged address control (prctl()) */
+#define NT_ARM_PRSTATUS 0x410 /* ARM general-purpose registers */
#define NT_ARC_V2 0x600 /* ARCv2 accumulator/extra registers */
#define NT_VMCOREDD 0x700 /* Vmcore Device Dump Note */
#define NT_MIPS_DSP 0x800 /* MIPS DSP ASE registers */
--
2.29.2


2021-01-28 00:01:29

by Dave Martin

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers

On Tue, Jan 19, 2021 at 02:06:36PM -0800, Andrei Vagin wrote:
> This is an alternative to NT_PRSTATUS that clobbers ip/r12 on AArch32,
> x7 on AArch64 when a tracee is stopped in syscall entry or syscall exit
> traps.
>
> Signed-off-by: Andrei Vagin <[email protected]>

This approach looks like it works, though I still think adding an option
for this under PTRACE_SETOPTIONS would be less intrusive.

Adding a shadow regset like this also looks like it would cause the gp
regs to be pointlessly be dumped twice in a core dump. Avoiding that
might require hacks in the core code...


> ---
> arch/arm64/kernel/ptrace.c | 39 ++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/elf.h | 1 +
> 2 files changed, 40 insertions(+)
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 1863f080cb07..b8e4c2ddf636 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -591,6 +591,15 @@ static int gpr_get(struct task_struct *target,
> return ret;
> }
>
> +static int gpr_get_full(struct task_struct *target,
> + const struct user_regset *regset,
> + struct membuf to)
> +{
> + struct user_pt_regs *uregs = &task_pt_regs(target)->user_regs;
> +
> + return membuf_write(&to, uregs, sizeof(*uregs));
> +}
> +
> static int gpr_set(struct task_struct *target, const struct user_regset *regset,
> unsigned int pos, unsigned int count,
> const void *kbuf, const void __user *ubuf)
> @@ -1088,6 +1097,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target, const struct
>
> enum aarch64_regset {
> REGSET_GPR,
> + REGSET_GPR_FULL,

If we go with this approach, "REGSET_GPR_RAW" might be a preferable
name. Both regs represent all the regs ("full"), but REGSET_GPR is
mangled by the kernel.

> REGSET_FPR,
> REGSET_TLS,
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> @@ -1119,6 +1129,14 @@ static const struct user_regset aarch64_regsets[] = {
> .regset_get = gpr_get,
> .set = gpr_set
> },
> + [REGSET_GPR_FULL] = {
> + .core_note_type = NT_ARM_PRSTATUS,

Similarly, something like NT_ARM_PRSTATUS_RAW or similar.

> + .n = sizeof(struct user_pt_regs) / sizeof(u64),
> + .size = sizeof(u64),
> + .align = sizeof(u64),
> + .regset_get = gpr_get_full,
> + .set = gpr_set
> + },
> [REGSET_FPR] = {
> .core_note_type = NT_PRFPREG,
> .n = sizeof(struct user_fpsimd_state) / sizeof(u32),
> @@ -1225,6 +1243,7 @@ static const struct user_regset_view user_aarch64_view = {
> #ifdef CONFIG_COMPAT
> enum compat_regset {
> REGSET_COMPAT_GPR,
> + REGSET_COMPAT_GPR_FULL,
> REGSET_COMPAT_VFP,
> };
>
> @@ -1285,6 +1304,18 @@ static int compat_gpr_get(struct task_struct *target,
> return 0;
> }
>
> +/* compat_gpr_get_full doesn't overwrite x12 like compat_gpr_get. */
> +static int compat_gpr_get_full(struct task_struct *target,
> + const struct user_regset *regset,
> + struct membuf to)
> +{
> + int i = 0;
> +
> + while (to.left)
> + membuf_store(&to, compat_get_user_reg(target, i++));
> + return 0;
> +}
> +
> static int compat_gpr_set(struct task_struct *target,
> const struct user_regset *regset,
> unsigned int pos, unsigned int count,
> @@ -1435,6 +1466,14 @@ static const struct user_regset aarch32_regsets[] = {
> .regset_get = compat_gpr_get,
> .set = compat_gpr_set
> },
> + [REGSET_COMPAT_GPR_FULL] = {
> + .core_note_type = NT_ARM_PRSTATUS,
> + .n = COMPAT_ELF_NGREG,
> + .size = sizeof(compat_elf_greg_t),
> + .align = sizeof(compat_elf_greg_t),
> + .regset_get = compat_gpr_get_full,
> + .set = compat_gpr_set
> + },
> [REGSET_COMPAT_VFP] = {
> .core_note_type = NT_ARM_VFP,
> .n = VFP_STATE_SIZE / sizeof(compat_ulong_t),
> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
> index 30f68b42eeb5..a2086d19263a 100644
> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -426,6 +426,7 @@ typedef struct elf64_shdr {
> #define NT_ARM_PACA_KEYS 0x407 /* ARM pointer authentication address keys */
> #define NT_ARM_PACG_KEYS 0x408 /* ARM pointer authentication generic key */
> #define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* arm64 tagged address control (prctl()) */

What happened to 0x40a..0x40f?

[...]

Cheers
---Dave

2021-01-29 10:08:17

by Andrei Vagin

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers

On Wed, Jan 27, 2021 at 02:53:07PM +0000, Dave Martin wrote:
> On Tue, Jan 19, 2021 at 02:06:36PM -0800, Andrei Vagin wrote:
> > This is an alternative to NT_PRSTATUS that clobbers ip/r12 on AArch32,
> > x7 on AArch64 when a tracee is stopped in syscall entry or syscall exit
> > traps.
> >
> > Signed-off-by: Andrei Vagin <[email protected]>
>
> This approach looks like it works, though I still think adding an option
> for this under PTRACE_SETOPTIONS would be less intrusive.

Dave, thank you for the feedback. I will prepare a patch with an option
and then we will see what looks better.

>
> Adding a shadow regset like this also looks like it would cause the gp
> regs to be pointlessly be dumped twice in a core dump. Avoiding that
> might require hacks in the core code...
>
>
> > ---
> > arch/arm64/kernel/ptrace.c | 39 ++++++++++++++++++++++++++++++++++++++
> > include/uapi/linux/elf.h | 1 +
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > index 1863f080cb07..b8e4c2ddf636 100644
> > --- a/arch/arm64/kernel/ptrace.c
> > +++ b/arch/arm64/kernel/ptrace.c
> > @@ -591,6 +591,15 @@ static int gpr_get(struct task_struct *target,
> > return ret;
> > }
> >
> > +static int gpr_get_full(struct task_struct *target,
> > + const struct user_regset *regset,
> > + struct membuf to)
> > +{
> > + struct user_pt_regs *uregs = &task_pt_regs(target)->user_regs;
> > +
> > + return membuf_write(&to, uregs, sizeof(*uregs));
> > +}
> > +
> > static int gpr_set(struct task_struct *target, const struct user_regset *regset,
> > unsigned int pos, unsigned int count,
> > const void *kbuf, const void __user *ubuf)
> > @@ -1088,6 +1097,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target, const struct
> >
> > enum aarch64_regset {
> > REGSET_GPR,
> > + REGSET_GPR_FULL,
>
> If we go with this approach, "REGSET_GPR_RAW" might be a preferable
> name. Both regs represent all the regs ("full"), but REGSET_GPR is
> mangled by the kernel.

I agree that REGSET_GPR_RAW looks better in this case.

>
> > REGSET_FPR,
> > REGSET_TLS,
> > #ifdef CONFIG_HAVE_HW_BREAKPOINT
> > @@ -1119,6 +1129,14 @@ static const struct user_regset aarch64_regsets[] = {
> > .regset_get = gpr_get,
> > .set = gpr_set
> > },
> > + [REGSET_GPR_FULL] = {
> > + .core_note_type = NT_ARM_PRSTATUS,

...

> > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
> > index 30f68b42eeb5..a2086d19263a 100644
> > --- a/include/uapi/linux/elf.h
> > +++ b/include/uapi/linux/elf.h
> > @@ -426,6 +426,7 @@ typedef struct elf64_shdr {
> > #define NT_ARM_PACA_KEYS 0x407 /* ARM pointer authentication address keys */
> > #define NT_ARM_PACG_KEYS 0x408 /* ARM pointer authentication generic key */
> > #define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* arm64 tagged address control (prctl()) */
>
> What happened to 0x40a..0x40f?

shame on me :)

>
> [...]
>
> Cheers
> ---Dave