From: Andrei Vagin <avagin@gmail.com> To: Will Deacon <will@kernel.org>, Catalin Marinas <catalin.marinas@arm.com> Cc: Oleg Nesterov <oleg@redhat.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andrei Vagin <avagin@gmail.com> Subject: [PATCH 1/3] arm64/ptrace: don't clobber task registers on syscall entry/exit traps Date: Tue, 19 Jan 2021 14:06:35 -0800 [thread overview] Message-ID: <20210119220637.494476-2-avagin@gmail.com> (raw) In-Reply-To: <20210119220637.494476-1-avagin@gmail.com> ip/r12 for AArch32 and x7 for AArch64 is used to indicate whether or not the stop has been signalled from syscall entry or syscall exit. This means that: - Any writes by the tracer to this register during the stop are ignored/discarded. - The actual value of the register is not available during the stop, so the tracer cannot save it and restore it later. Right now, these registers are clobbered in tracehook_report_syscall. This change moves this logic to gpr_get and compat_gpr_get where registers are copied into a user-space buffer. This will allow to change these registers and to introduce a new NT_ARM_PRSTATUS command to get the full set of registers. Signed-off-by: Andrei Vagin <avagin@gmail.com> --- arch/arm64/include/asm/ptrace.h | 5 ++ arch/arm64/kernel/ptrace.c | 104 +++++++++++++++++++------------- 2 files changed, 67 insertions(+), 42 deletions(-) diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index e58bca832dff..0a9552b4f61e 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -170,6 +170,11 @@ static inline unsigned long pstate_to_compat_psr(const unsigned long pstate) return psr; } +enum ptrace_syscall_dir { + PTRACE_SYSCALL_ENTER = 0, + PTRACE_SYSCALL_EXIT, +}; + /* * This struct defines the way the registers are stored on the stack during an * exception. Note that sizeof(struct pt_regs) has to be a multiple of 16 (for diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 8ac487c84e37..1863f080cb07 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -40,6 +40,7 @@ #include <asm/syscall.h> #include <asm/traps.h> #include <asm/system_misc.h> +#include <asm/ptrace.h> #define CREATE_TRACE_POINTS #include <trace/events/syscalls.h> @@ -561,7 +562,33 @@ static int gpr_get(struct task_struct *target, struct membuf to) { struct user_pt_regs *uregs = &task_pt_regs(target)->user_regs; - return membuf_write(&to, uregs, sizeof(*uregs)); + unsigned long saved_reg; + int ret; + + /* + * We have some ABI weirdness here 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 the general + * purpose register x7. + */ + switch (target->ptrace_message) { + case PTRACE_EVENTMSG_SYSCALL_ENTRY: + saved_reg = uregs->regs[7]; + uregs->regs[7] = PTRACE_SYSCALL_ENTER; + break; + case PTRACE_EVENTMSG_SYSCALL_EXIT: + saved_reg = uregs->regs[7]; + uregs->regs[7] = PTRACE_SYSCALL_EXIT; + break; + } + + ret = membuf_write(&to, uregs, sizeof(*uregs)); + + if (target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY || + target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) + uregs->regs[7] = saved_reg; + + return ret; } static int gpr_set(struct task_struct *target, const struct user_regset *regset, @@ -1221,10 +1248,40 @@ static int compat_gpr_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { + compat_ulong_t r12; + bool overwrite_r12; int i = 0; - while (to.left) - membuf_store(&to, compat_get_user_reg(target, i++)); + /* + * We have some ABI weirdness here 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 the + * general purpose register r12. + */ + switch (target->ptrace_message) { + case PTRACE_EVENTMSG_SYSCALL_ENTRY: + r12 = PTRACE_SYSCALL_ENTER; + overwrite_r12 = true; + break; + case PTRACE_EVENTMSG_SYSCALL_EXIT: + r12 = PTRACE_SYSCALL_EXIT; + overwrite_r12 = true; + break; + default: + overwrite_r12 = false; + break; + } + + while (to.left) { + compat_ulong_t val; + + if (!overwrite_r12 || i != 12) + val = compat_get_user_reg(target, i++); + else + val = r12; + membuf_store(&to, val); + } + return 0; } @@ -1740,53 +1797,16 @@ long arch_ptrace(struct task_struct *child, long request, return ptrace_request(child, request, addr, data); } -enum ptrace_syscall_dir { - PTRACE_SYSCALL_ENTER = 0, - PTRACE_SYSCALL_EXIT, -}; - static void tracehook_report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) { - int regno; - unsigned long saved_reg; - - /* - * We have some ABI weirdness here 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 means that: - * - * - Any writes by the tracer to this register during the stop are - * ignored/discarded. - * - * - The actual value of the register is not available during the stop, - * so the tracer cannot save it and restore it later. - * - * - Syscall stops behave differently to seccomp and pseudo-step traps - * (the latter do not nobble any registers). - */ - regno = (is_compat_task() ? 12 : 7); - saved_reg = regs->regs[regno]; - regs->regs[regno] = dir; - if (dir == PTRACE_SYSCALL_ENTER) { if (tracehook_report_syscall_entry(regs)) forget_syscall(regs); - regs->regs[regno] = saved_reg; - } else if (!test_thread_flag(TIF_SINGLESTEP)) { - tracehook_report_syscall_exit(regs, 0); - regs->regs[regno] = saved_reg; } else { - regs->regs[regno] = saved_reg; + int singlestep = test_thread_flag(TIF_SINGLESTEP); - /* - * Signal a pseudo-step exception since we are stepping but - * tracer modifications to the registers may have rewound the - * state machine. - */ - tracehook_report_syscall_exit(regs, 1); + tracehook_report_syscall_exit(regs, singlestep); } } -- 2.29.2
WARNING: multiple messages have this Message-ID (diff)
From: Andrei Vagin <avagin@gmail.com> To: Will Deacon <will@kernel.org>, Catalin Marinas <catalin.marinas@arm.com> Cc: Andrei Vagin <avagin@gmail.com>, Oleg Nesterov <oleg@redhat.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/3] arm64/ptrace: don't clobber task registers on syscall entry/exit traps Date: Tue, 19 Jan 2021 14:06:35 -0800 [thread overview] Message-ID: <20210119220637.494476-2-avagin@gmail.com> (raw) In-Reply-To: <20210119220637.494476-1-avagin@gmail.com> ip/r12 for AArch32 and x7 for AArch64 is used to indicate whether or not the stop has been signalled from syscall entry or syscall exit. This means that: - Any writes by the tracer to this register during the stop are ignored/discarded. - The actual value of the register is not available during the stop, so the tracer cannot save it and restore it later. Right now, these registers are clobbered in tracehook_report_syscall. This change moves this logic to gpr_get and compat_gpr_get where registers are copied into a user-space buffer. This will allow to change these registers and to introduce a new NT_ARM_PRSTATUS command to get the full set of registers. Signed-off-by: Andrei Vagin <avagin@gmail.com> --- arch/arm64/include/asm/ptrace.h | 5 ++ arch/arm64/kernel/ptrace.c | 104 +++++++++++++++++++------------- 2 files changed, 67 insertions(+), 42 deletions(-) diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index e58bca832dff..0a9552b4f61e 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -170,6 +170,11 @@ static inline unsigned long pstate_to_compat_psr(const unsigned long pstate) return psr; } +enum ptrace_syscall_dir { + PTRACE_SYSCALL_ENTER = 0, + PTRACE_SYSCALL_EXIT, +}; + /* * This struct defines the way the registers are stored on the stack during an * exception. Note that sizeof(struct pt_regs) has to be a multiple of 16 (for diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 8ac487c84e37..1863f080cb07 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -40,6 +40,7 @@ #include <asm/syscall.h> #include <asm/traps.h> #include <asm/system_misc.h> +#include <asm/ptrace.h> #define CREATE_TRACE_POINTS #include <trace/events/syscalls.h> @@ -561,7 +562,33 @@ static int gpr_get(struct task_struct *target, struct membuf to) { struct user_pt_regs *uregs = &task_pt_regs(target)->user_regs; - return membuf_write(&to, uregs, sizeof(*uregs)); + unsigned long saved_reg; + int ret; + + /* + * We have some ABI weirdness here 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 the general + * purpose register x7. + */ + switch (target->ptrace_message) { + case PTRACE_EVENTMSG_SYSCALL_ENTRY: + saved_reg = uregs->regs[7]; + uregs->regs[7] = PTRACE_SYSCALL_ENTER; + break; + case PTRACE_EVENTMSG_SYSCALL_EXIT: + saved_reg = uregs->regs[7]; + uregs->regs[7] = PTRACE_SYSCALL_EXIT; + break; + } + + ret = membuf_write(&to, uregs, sizeof(*uregs)); + + if (target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY || + target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) + uregs->regs[7] = saved_reg; + + return ret; } static int gpr_set(struct task_struct *target, const struct user_regset *regset, @@ -1221,10 +1248,40 @@ static int compat_gpr_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { + compat_ulong_t r12; + bool overwrite_r12; int i = 0; - while (to.left) - membuf_store(&to, compat_get_user_reg(target, i++)); + /* + * We have some ABI weirdness here 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 the + * general purpose register r12. + */ + switch (target->ptrace_message) { + case PTRACE_EVENTMSG_SYSCALL_ENTRY: + r12 = PTRACE_SYSCALL_ENTER; + overwrite_r12 = true; + break; + case PTRACE_EVENTMSG_SYSCALL_EXIT: + r12 = PTRACE_SYSCALL_EXIT; + overwrite_r12 = true; + break; + default: + overwrite_r12 = false; + break; + } + + while (to.left) { + compat_ulong_t val; + + if (!overwrite_r12 || i != 12) + val = compat_get_user_reg(target, i++); + else + val = r12; + membuf_store(&to, val); + } + return 0; } @@ -1740,53 +1797,16 @@ long arch_ptrace(struct task_struct *child, long request, return ptrace_request(child, request, addr, data); } -enum ptrace_syscall_dir { - PTRACE_SYSCALL_ENTER = 0, - PTRACE_SYSCALL_EXIT, -}; - static void tracehook_report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) { - int regno; - unsigned long saved_reg; - - /* - * We have some ABI weirdness here 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 means that: - * - * - Any writes by the tracer to this register during the stop are - * ignored/discarded. - * - * - The actual value of the register is not available during the stop, - * so the tracer cannot save it and restore it later. - * - * - Syscall stops behave differently to seccomp and pseudo-step traps - * (the latter do not nobble any registers). - */ - regno = (is_compat_task() ? 12 : 7); - saved_reg = regs->regs[regno]; - regs->regs[regno] = dir; - if (dir == PTRACE_SYSCALL_ENTER) { if (tracehook_report_syscall_entry(regs)) forget_syscall(regs); - regs->regs[regno] = saved_reg; - } else if (!test_thread_flag(TIF_SINGLESTEP)) { - tracehook_report_syscall_exit(regs, 0); - regs->regs[regno] = saved_reg; } else { - regs->regs[regno] = saved_reg; + int singlestep = test_thread_flag(TIF_SINGLESTEP); - /* - * Signal a pseudo-step exception since we are stepping but - * tracer modifications to the registers may have rewound the - * state machine. - */ - tracehook_report_syscall_exit(regs, 1); + tracehook_report_syscall_exit(regs, singlestep); } } -- 2.29.2 _______________________________________________ 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-01-19 22:12 UTC|newest] Thread overview: 24+ 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 ` Andrei Vagin [this message] 2021-01-19 22:06 ` [PATCH 1/3] arm64/ptrace: don't clobber task registers on syscall entry/exit traps 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 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 2021-02-01 19:40 [PATCH 0/3 v2] " 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
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=20210119220637.494476-2-avagin@gmail.com \ --to=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: 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.