linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: linux-arm-kernel@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Luis Machado <luis.machado@linaro.org>,
	Kees Cook <keescook@chromium.org>, Will Deacon <will@kernel.org>,
	catalin.marinas@arm.com, Keno Fischer <keno@juliacomputing.com>,
	kernel-team@android.com
Subject: [PATCH v3 4/7] arm64: ptrace: Add a comment describing our syscall entry/exit trap ABI
Date: Fri, 10 Jul 2020 14:06:59 +0100	[thread overview]
Message-ID: <20200710130702.30658-5-will@kernel.org> (raw)
In-Reply-To: <20200710130702.30658-1-will@kernel.org>

Our tracehook logic for syscall entry/exit raises a SIGTRAP back to the
tracer following a ptrace request such as PTRACE_SYSCALL. As part of this
procedure, we clobber the reported value of one of the tracee's general
purpose registers (x7 for native tasks, r12 for compat) to indicate
whether the stop occurred on syscall entry or exit. This is a slightly
unfortunate ABI, as it prevents the tracer from accessing the real
register value and is at odds with other similar stops such as seccomp
traps.

Since we're stuck with this ABI, expand the comment in our tracehook
logic to acknowledge the issue and descibe the behaviour in more detail.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Luis Machado <luis.machado@linaro.org>
Reported-by: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/ptrace.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 22f9053b55b6..89fbee3991a2 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1811,8 +1811,20 @@ static void tracehook_report_syscall(struct pt_regs *regs,
 	unsigned long saved_reg;
 
 	/*
-	 * A scratch register (ip(r12) on AArch32, x7 on AArch64) is
-	 * used to denote syscall entry/exit:
+	 * 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];
-- 
2.27.0.383.g050319c2ae-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2020-07-10 13:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-10 13:06 [PATCH v3 0/7] arm64: Fix single-step handling and syscall tracing Will Deacon
2020-07-10 13:06 ` [PATCH v3 1/7] arm64: ptrace: Consistently use pseudo-singlestep exceptions Will Deacon
2020-07-16  0:27   ` Sasha Levin
2020-07-10 13:06 ` [PATCH v3 2/7] arm64: ptrace: Override SPSR.SS when single-stepping is enabled Will Deacon
2020-07-16  0:27   ` Sasha Levin
2020-07-10 13:06 ` [PATCH v3 3/7] arm64: compat: Ensure upper 32 bits of x0 are zero on syscall return Will Deacon
2020-07-16  0:27   ` Sasha Levin
2020-07-10 13:06 ` Will Deacon [this message]
2020-07-10 13:07 ` [PATCH v3 5/7] arm64: syscall: Expand the comment about ptrace and syscall(-1) Will Deacon
2020-07-10 13:07 ` [PATCH v3 6/7] arm64: ptrace: Use NO_SYSCALL instead of -1 in syscall_trace_enter() Will Deacon
2020-07-10 16:04   ` Kees Cook
2020-07-10 16:11     ` Will Deacon
2020-07-13  2:32       ` Kees Cook
2020-07-10 13:07 ` [PATCH v3 7/7] arm64: Use test_tsk_thread_flag() for checking TIF_SINGLESTEP Will Deacon
2020-07-14 11:57 ` [PATCH v3 0/7] arm64: Fix single-step handling and syscall tracing Luis Machado
2020-07-15 12:25 ` Luis Machado

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=20200710130702.30658-5-will@kernel.org \
    --to=will@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=keescook@chromium.org \
    --cc=keno@juliacomputing.com \
    --cc=kernel-team@android.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=luis.machado@linaro.org \
    --cc=mark.rutland@arm.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).