From: Will Deacon <will@kernel.org> To: Andrei Vagin <avagin@gmail.com> Cc: Catalin Marinas <catalin.marinas@arm.com>, Oleg Nesterov <oleg@redhat.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Anthony Steinhauser <asteinhauser@google.com>, Dave Martin <Dave.Martin@arm.com>, Keno Fischer <keno@juliacomputing.com> Subject: Re: [PATCH 2/3] arm64/ptrace: introduce PTRACE_O_ARM64_RAW_REGS Date: Thu, 4 Feb 2021 15:36:15 +0000 [thread overview] Message-ID: <20210204153615.GB21058@willie-the-truck> (raw) In-Reply-To: <20210201194012.524831-3-avagin@gmail.com> On Mon, Feb 01, 2021 at 11:40:11AM -0800, Andrei Vagin wrote: > We have some ABI weirdness in the way that we handle syscall > exit stops because we indicate whether or not the stop has been > signalled from syscall entry or syscall exit by clobbering a general > purpose register (ip/r12 for AArch32, x7 for AArch64) in the tracee > and restoring its old value after the stop. > > This behavior was inherited from ARM and it isn't common for other > architectures. Now, we have PTRACE_GET_SYSCALL_INFO that gives all > required information about system calls, so the hack with clobbering > registers isn't needed anymore. > > This change adds the new ptrace option PTRACE_O_ARM64_RAW_REGS. If it > is set, PTRACE_GETREGSET returns values of all registers without > clobbering r12 or x7 and PTRACE_SETREGSE sets all registers even if a > process has been stopped in syscall-enter or syscall-exit. > > Signed-off-by: Andrei Vagin <avagin@gmail.com> > --- > arch/arm64/include/uapi/asm/ptrace.h | 4 ++ > arch/arm64/kernel/ptrace.c | 70 ++++++++++++++++------------ > include/linux/ptrace.h | 1 + > include/uapi/linux/ptrace.h | 9 +++- > 4 files changed, 52 insertions(+), 32 deletions(-) Please split this up so that the arm64-specific changes are separate to the core changes. > diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h > index 83ee45fa634b..bcc8c362ddd9 100644 > --- a/include/uapi/linux/ptrace.h > +++ b/include/uapi/linux/ptrace.h > @@ -7,6 +7,7 @@ > /* has the defines to get at the registers. */ > > #include <linux/types.h> > +#include <asm/ptrace.h> > > #define PTRACE_TRACEME 0 > #define PTRACE_PEEKTEXT 1 > @@ -137,8 +138,14 @@ struct ptrace_syscall_info { > #define PTRACE_O_EXITKILL (1 << 20) > #define PTRACE_O_SUSPEND_SECCOMP (1 << 21) > > +/* (1<<28) is reserved for arch specific options. */ > +#ifndef _PTRACE_O_ARCH_OPTIONS > +#define _PTRACE_O_ARCH_OPTIONS 0 > +#endif It seems a bit fragile to rely on a comment here to define the user ABI; why not define _PTRACE_O_ARCH_OPTIONS to the right value unconditionally? Also, it seems as though we immediately burn this bit on arm64, so if we ever wanted another option we'd have to come back here and allocate another bit. Could we do better, e.g. by calling into an arch hook (arch_ptrace_setoptions()) and passing the 'addr' parameter? How do other architectures manage this sort of thing? I'm wondering whether a separate regset containing just "real x7" and orig_x0 would be preferable after all... Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org> To: Andrei Vagin <avagin@gmail.com> Cc: Anthony Steinhauser <asteinhauser@google.com>, Catalin Marinas <catalin.marinas@arm.com>, Oleg Nesterov <oleg@redhat.com>, linux-kernel@vger.kernel.org, Keno Fischer <keno@juliacomputing.com>, linux-api@vger.kernel.org, Dave Martin <Dave.Martin@arm.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] arm64/ptrace: introduce PTRACE_O_ARM64_RAW_REGS Date: Thu, 4 Feb 2021 15:36:15 +0000 [thread overview] Message-ID: <20210204153615.GB21058@willie-the-truck> (raw) In-Reply-To: <20210201194012.524831-3-avagin@gmail.com> On Mon, Feb 01, 2021 at 11:40:11AM -0800, Andrei Vagin wrote: > We have some ABI weirdness in the way that we handle syscall > exit stops because we indicate whether or not the stop has been > signalled from syscall entry or syscall exit by clobbering a general > purpose register (ip/r12 for AArch32, x7 for AArch64) in the tracee > and restoring its old value after the stop. > > This behavior was inherited from ARM and it isn't common for other > architectures. Now, we have PTRACE_GET_SYSCALL_INFO that gives all > required information about system calls, so the hack with clobbering > registers isn't needed anymore. > > This change adds the new ptrace option PTRACE_O_ARM64_RAW_REGS. If it > is set, PTRACE_GETREGSET returns values of all registers without > clobbering r12 or x7 and PTRACE_SETREGSE sets all registers even if a > process has been stopped in syscall-enter or syscall-exit. > > Signed-off-by: Andrei Vagin <avagin@gmail.com> > --- > arch/arm64/include/uapi/asm/ptrace.h | 4 ++ > arch/arm64/kernel/ptrace.c | 70 ++++++++++++++++------------ > include/linux/ptrace.h | 1 + > include/uapi/linux/ptrace.h | 9 +++- > 4 files changed, 52 insertions(+), 32 deletions(-) Please split this up so that the arm64-specific changes are separate to the core changes. > diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h > index 83ee45fa634b..bcc8c362ddd9 100644 > --- a/include/uapi/linux/ptrace.h > +++ b/include/uapi/linux/ptrace.h > @@ -7,6 +7,7 @@ > /* has the defines to get at the registers. */ > > #include <linux/types.h> > +#include <asm/ptrace.h> > > #define PTRACE_TRACEME 0 > #define PTRACE_PEEKTEXT 1 > @@ -137,8 +138,14 @@ struct ptrace_syscall_info { > #define PTRACE_O_EXITKILL (1 << 20) > #define PTRACE_O_SUSPEND_SECCOMP (1 << 21) > > +/* (1<<28) is reserved for arch specific options. */ > +#ifndef _PTRACE_O_ARCH_OPTIONS > +#define _PTRACE_O_ARCH_OPTIONS 0 > +#endif It seems a bit fragile to rely on a comment here to define the user ABI; why not define _PTRACE_O_ARCH_OPTIONS to the right value unconditionally? Also, it seems as though we immediately burn this bit on arm64, so if we ever wanted another option we'd have to come back here and allocate another bit. Could we do better, e.g. by calling into an arch hook (arch_ptrace_setoptions()) and passing the 'addr' parameter? How do other architectures manage this sort of thing? I'm wondering whether a separate regset containing just "real x7" and orig_x0 would be preferable after all... Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-02-04 15:41 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-01 19:40 [PATCH 0/3 v2] arm64/ptrace: allow to get all registers on syscall traps Andrei Vagin 2021-02-01 19:40 ` Andrei Vagin 2021-02-01 19:40 ` [PATCH 1/3] arm64/ptrace: don't clobber task registers on syscall entry/exit traps Andrei Vagin 2021-02-01 19:40 ` Andrei Vagin 2021-02-04 15:23 ` Will Deacon 2021-02-04 15:23 ` Will Deacon 2021-02-04 16:41 ` Dave Martin 2021-02-04 16:41 ` Dave Martin 2021-02-25 16:00 ` Andrei Vagin 2021-02-25 16:00 ` Andrei Vagin 2021-02-01 19:40 ` [PATCH 2/3] arm64/ptrace: introduce PTRACE_O_ARM64_RAW_REGS Andrei Vagin 2021-02-01 19:40 ` Andrei Vagin 2021-02-04 15:36 ` Will Deacon [this message] 2021-02-04 15:36 ` Will Deacon 2021-02-08 18:31 ` Andrei Vagin 2021-02-08 18:31 ` Andrei Vagin 2021-02-01 19:40 ` [PATCH 3/3] selftest/arm64/ptrace: add tests for PTRACE_O_ARM64_RAW_REGS Andrei Vagin 2021-02-01 19:40 ` Andrei Vagin 2021-02-04 15:40 ` Will Deacon 2021-02-04 15:40 ` Will Deacon 2021-02-10 20:54 ` Kees Cook 2021-02-10 20:54 ` Kees Cook 2021-02-02 0:11 ` [PATCH 0/3 v2] arm64/ptrace: allow to get all registers on syscall traps Keno Fischer 2021-02-02 0:11 ` Keno Fischer 2021-02-08 18:37 ` Andrei Vagin 2021-02-08 18:37 ` Andrei Vagin 2021-02-08 19:18 ` Keno Fischer 2021-02-08 19:18 ` Keno Fischer 2021-02-04 14:53 ` Will Deacon 2021-02-04 14:53 ` Will Deacon
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=20210204153615.GB21058@willie-the-truck \ --to=will@kernel.org \ --cc=Dave.Martin@arm.com \ --cc=asteinhauser@google.com \ --cc=avagin@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=keno@juliacomputing.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=oleg@redhat.com \ /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: linkBe 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.