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
