live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: madvenka@linux.microsoft.com
To: mark.rutland@arm.com, broonie@kernel.org, jpoimboe@redhat.com,
	jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	madvenka@linux.microsoft.com
Subject: [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable
Date: Tue, 30 Mar 2021 14:09:54 -0500	[thread overview]
Message-ID: <20210330190955.13707-4-madvenka@linux.microsoft.com> (raw)
In-Reply-To: <20210330190955.13707-1-madvenka@linux.microsoft.com>

From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>

When CONFIG_DYNAMIC_FTRACE_WITH_REGS is enabled and tracing is activated
for a function, the ftrace infrastructure is called for the function at
the very beginning. Ftrace creates two frames:

	- One for the traced function

	- One for the caller of the traced function

That gives a reliable stack trace while executing in the ftrace code. When
ftrace returns to the traced function, the frames are popped and everything
is back to normal.

However, in cases like live patch, a tracer function may redirect execution
to a different function when it returns. A stack trace taken while still in
the tracer function will not show the target function. The target function
is the real function that we want to track.

So, if an FTRACE frame is detected on the stack, just mark the stack trace
as unreliable. The detection is done by checking the return PC against
FTRACE trampolines.

Also, the Function Graph Tracer modifies the return address of a traced
function to a return trampoline to gather tracing data on function return.
Stack traces taken from that trampoline and functions it calls are
unreliable as the original return address may not be available in
that context. Mark the stack trace unreliable accordingly.

Signed-off-by: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
---
 arch/arm64/kernel/entry-ftrace.S | 10 ++++++
 arch/arm64/kernel/stacktrace.c   | 57 ++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index b3e4f9a088b1..5373f88b4c44 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -95,6 +95,16 @@ SYM_CODE_START(ftrace_common)
 SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
 	bl	ftrace_stub
 
+	/*
+	 * The only call in the FTRACE trampoline code is above. The above
+	 * instruction is patched to call a tracer function. Its return
+	 * address is below (ftrace_graph_call). In a stack trace taken from
+	 * a tracer function, ftrace_graph_call() will show. The unwinder
+	 * checks this for reliable stack trace. Please see the comments
+	 * in stacktrace.c. If another call is added in the FTRACE
+	 * trampoline code, the special_functions[] array in stacktrace.c
+	 * must be updated.
+	 */
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL) // ftrace_graph_caller();
 	nop				// If enabled, this will be replaced
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 7662f57d3e88..8b493a90c9f3 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -51,6 +51,52 @@ struct function_range {
  * unreliable. Breakpoints are used for executing probe code. Stack traces
  * taken while in the probe code will show an EL1 frame and will be considered
  * unreliable. This is correct behavior.
+ *
+ * FTRACE
+ * ======
+ *
+ * When CONFIG_DYNAMIC_FTRACE_WITH_REGS is enabled, the FTRACE trampoline code
+ * is called from a traced function even before the frame pointer prolog.
+ * FTRACE sets up two stack frames (one for the traced function and one for
+ * its caller) so that the unwinder can provide a sensible stack trace for
+ * any tracer function called from the FTRACE trampoline code.
+ *
+ * There are two cases where the stack trace is not reliable.
+ *
+ * (1) The task gets preempted before the two frames are set up. Preemption
+ *     involves an interrupt which is an EL1 exception. The unwinder already
+ *     handles EL1 exceptions.
+ *
+ * (2) The tracer function that gets called by the FTRACE trampoline code
+ *     changes the return PC (e.g., livepatch).
+ *
+ *     Not all tracer functions do that. But to err on the side of safety,
+ *     consider the stack trace as unreliable in all cases.
+ *
+ * When Function Graph Tracer is used, FTRACE modifies the return address of
+ * the traced function in its stack frame to an FTRACE return trampoline
+ * (return_to_handler). When the traced function returns, control goes to
+ * return_to_handler. return_to_handler calls FTRACE to gather tracing data
+ * and to obtain the original return address. Then, return_to_handler returns
+ * to the original return address.
+ *
+ * There are two cases to consider from a stack trace reliability point of
+ * view:
+ *
+ * (1) Stack traces taken within the traced function (and functions that get
+ *     called from there) will show return_to_handler instead of the original
+ *     return address. The original return address can be obtained from FTRACE.
+ *     The unwinder already obtains it and modifies the return PC in its copy
+ *     of the stack frame to the original return address. So, this is handled.
+ *
+ * (2) return_to_handler calls FTRACE as mentioned before. FTRACE discards
+ *     the record of the original return address along the way as it does not
+ *     need to maintain it anymore. This means that the unwinder cannot get
+ *     the original return address beyond that point while the task is still
+ *     executing in return_to_handler. So, consider the stack trace unreliable
+ *     if return_to_handler is detected on the stack.
+ *
+ * NOTE: The unwinder must do (1) before (2).
  */
 static struct function_range	special_functions[] = {
 	/*
@@ -64,6 +110,17 @@ static struct function_range	special_functions[] = {
 	{ (unsigned long) el1_fiq_invalid, 0 },
 	{ (unsigned long) el1_error_invalid, 0 },
 
+	/*
+	 * FTRACE trampolines.
+	 */
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	{ (unsigned long) &ftrace_graph_call, 0 },
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	{ (unsigned long) ftrace_graph_caller, 0 },
+	{ (unsigned long) return_to_handler, 0 },
+#endif
+#endif
+
 	{ 0, 0 }
 };
 
-- 
2.25.1


  parent reply	other threads:[~2021-03-30 19:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <77bd5edeea72d44533c769b1e8c0fea7a9d7eb3a>
2021-03-30 19:09 ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks madvenka
2021-03-30 19:09   ` [RFC PATCH v1 1/4] arm64: Implement infrastructure for " madvenka
2021-04-01 15:27     ` Mark Brown
2021-04-01 17:44       ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 2/4] arm64: Mark a stack trace unreliable if an EL1 exception frame is detected madvenka
2021-04-01 17:21     ` Mark Brown
2021-03-30 19:09   ` madvenka [this message]
2021-04-01 14:27     ` [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable Mark Brown
2021-04-01 17:43       ` Madhavan T. Venkataraman
2021-04-01 18:28         ` Mark Brown
2021-04-01 18:40           ` Madhavan T. Venkataraman
2021-04-01 18:53             ` Madhavan T. Venkataraman
2021-04-01 19:47               ` Madhavan T. Venkataraman
2021-04-06 11:02                 ` Mark Brown
2021-04-01 17:48       ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 4/4] arm64: Mark stack trace as unreliable if kretprobed functions are present madvenka
2021-04-01 17:23     ` Mark Brown
2021-04-03 17:01   ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks Josh Poimboeuf
2021-04-04  3:29     ` Madhavan T. Venkataraman
2021-04-05 13:24       ` Masami Hiramatsu
2021-04-05 13:46         ` Madhavan T. Venkataraman
2021-04-05 14:56         ` Madhavan T. Venkataraman
2021-04-05 17:12           ` Madhavan T. Venkataraman
2021-04-05 23:39             ` Masami Hiramatsu
2021-04-05 23:40           ` Masami Hiramatsu

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=20210330190955.13707-4-madvenka@linux.microsoft.com \
    --to=madvenka@linux.microsoft.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=jpoimboe@redhat.com \
    --cc=jthierry@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=will@kernel.org \
    --subject='Re: [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable' \
    /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

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).