From: Dave Martin <Dave.Martin@arm.com>
To: Andrei Vagin <avagin@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers
Date: Wed, 27 Jan 2021 14:53:07 +0000 [thread overview]
Message-ID: <20210127145304.GC13952@arm.com> (raw)
In-Reply-To: <20210119220637.494476-3-avagin@gmail.com>
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 <avagin@gmail.com>
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <Dave.Martin@arm.com>
To: Andrei Vagin <avagin@gmail.com>
Cc: Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Oleg Nesterov <oleg@redhat.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers
Date: Wed, 27 Jan 2021 14:53:07 +0000 [thread overview]
Message-ID: <20210127145304.GC13952@arm.com> (raw)
In-Reply-To: <20210119220637.494476-3-avagin@gmail.com>
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 <avagin@gmail.com>
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
next prev parent reply other threads:[~2021-01-27 15:18 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-19 22:06 [PATCH 0/3] arm64/ptrace: allow to get all registers on syscall traps Andrei Vagin
2021-01-19 22:06 ` Andrei Vagin
2021-01-19 22:06 ` [PATCH 1/3] arm64/ptrace: don't clobber task registers on syscall entry/exit traps Andrei Vagin
2021-01-19 22:06 ` Andrei Vagin
2021-01-27 15:14 ` Dave Martin
2021-01-27 15:14 ` Dave Martin
2021-01-19 22:06 ` [PATCH 2/3] arm64/ptrace: introduce NT_ARM_PRSTATUS to get a full set of registers Andrei Vagin
2021-01-19 22:06 ` Andrei Vagin
2021-01-27 14:53 ` Dave Martin [this message]
2021-01-27 14:53 ` Dave Martin
2021-01-29 7:56 ` Andrei Vagin
2021-01-29 7:56 ` Andrei Vagin
2021-01-19 22:06 ` [PATCH 3/3] selftest/arm64/ptrace: add a test for NT_ARM_PRSTATUS Andrei Vagin
2021-01-19 22:06 ` Andrei Vagin
2021-01-27 8:10 ` [PATCH 0/3] arm64/ptrace: allow to get all registers on syscall traps Andrei Vagin
2021-01-27 8:10 ` Andrei Vagin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210127145304.GC13952@arm.com \
--to=dave.martin@arm.com \
--cc=avagin@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.