live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Randy Dunlap <rdunlap@infradead.org>
To: Mark Brown <broonie@kernel.org>, linux-kernel@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Jiri Kosina <jikos@kernel.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>, Miroslav Benes <mbenes@suse.cz>,
	Petr Mladek <pmladek@suse.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-doc@vgert.kernel.org, live-patching@vger.kernel.org
Subject: Re: [PATCH v5 2/2] Documentation: livepatch: document reliable stacktrace
Date: Mon, 18 Jan 2021 11:25:03 -0800	[thread overview]
Message-ID: <e0243599-f650-9b8e-0ccc-68b6163668f3@infradead.org> (raw)
In-Reply-To: <20210118173954.36577-3-broonie@kernel.org>

Hi,

On 1/18/21 9:39 AM, Mark Brown wrote:
> From: Mark Rutland <mark.rutland@arm.com>
> 
> Add documentation for reliable stacktrace. This is intended to describe
> the semantics and to be an aid for implementing architecture support for
> HAVE_RELIABLE_STACKTRACE.
> 
> Unwinding is a subtle area, and architectures vary greatly in both
> implementation and the set of concerns that affect them, so I've tried
> to avoid making this too specific to any given architecture. I've used
> examples from both x86_64 and arm64 to explain corner cases in more
> detail, but I've tried to keep the descriptions sufficient for those who
> are unfamiliar with the particular architecture.
> 
> This document aims to give rationale for all the recommendations and
> requirements, since that makes it easier to spot nearby issues, or when
> a check happens to catch a few things at once.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> [Updates following review -- broonie]
> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  Documentation/livepatch/index.rst             |   1 +
>  .../livepatch/reliable-stacktrace.rst         | 309 ++++++++++++++++++
>  2 files changed, 310 insertions(+)
>  create mode 100644 Documentation/livepatch/reliable-stacktrace.rst

> diff --git a/Documentation/livepatch/reliable-stacktrace.rst b/Documentation/livepatch/reliable-stacktrace.rst
> new file mode 100644
> index 000000000000..fb123ee33403
> --- /dev/null
> +++ b/Documentation/livepatch/reliable-stacktrace.rst
> @@ -0,0 +1,309 @@

> +4.4 Rewriting of return addresses
> +---------------------------------
> +
> +Some trampolines temporarily modify the return address of a function in order
> +to intercept when that function returns with a return trampoline, e.g.
> +
> +* An ftrace trampoline may modify the return address so that function graph
> +  tracing can intercept returns.
> +
> +* A kprobes (or optprobes) trampoline may modify the return address so that
> +  kretprobes can intercept returns.
> +
> +When this happens, the original return address will not be in its usual
> +location. For trampolines which are not subject to live patching, where an
> +unwinder can reliably determine the original return address and no unwind state
> +is altered by the trampoline, the unwinder may report the original return
> +address in place of the trampoline and report this as reliable. Otherwise, an
> +unwinder must report these cases as unreliable.
> +
> +Special care is required when identifying the original return address, as this
> +information is not in a consistent location for the duration of the entry
> +trampoline or return trampoline. For example, considering the x86_64
> +'return_to_handler' return trampoline:
> +
> +.. code-block:: none
> +
> +   SYM_CODE_START(return_to_handler)
> +           UNWIND_HINT_EMPTY
> +           subq  $24, %rsp
> +
> +           /* Save the return values */
> +           movq %rax, (%rsp)
> +           movq %rdx, 8(%rsp)
> +           movq %rbp, %rdi
> +
> +           call ftrace_return_to_handler
> +
> +           movq %rax, %rdi
> +           movq 8(%rsp), %rdx
> +           movq (%rsp), %rax
> +           addq $24, %rsp
> +           JMP_NOSPEC rdi
> +   SYM_CODE_END(return_to_handler)
> +
> +While the traced function runs its return address points on the stack points to

too many "points" above.

Otherwise:

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> +the start of return_to_handler, and the original return address is stored in
> +the task's cur_ret_stack. During this time the unwinder can find the return
> +address using ftrace_graph_ret_addr().
> +
> +When the traced function returns to return_to_handler, there is no longer a
> +return address on the stack, though the original return address is still stored
> +in the task's cur_ret_stack. Within ftrace_return_to_handler(), the original
> +return address is removed from cur_ret_stack and is transiently moved
> +arbitrarily by the compiler before being returned in rax. The return_to_handler
> +trampoline moves this into rdi before jumping to it.
> +
> +Architectures might not always be able to unwind such sequences, such as when
> +ftrace_return_to_handler() has removed the address from cur_ret_stack, and the
> +location of the return address cannot be reliably determined.
> +
> +It is recommended that architectures unwind cases where return_to_handler has
> +not yet been returned to, but architectures are not required to unwind from the
> +middle of return_to_handler and can report this as unreliable. Architectures
> +are not required to unwind from other trampolines which modify the return
> +address.



-- 
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law

  reply	other threads:[~2021-01-18 19:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 17:39 [PATCH v5 0/2] Documentation: livepatch: Document reliable stacktrace and minor cleanup Mark Brown
2021-01-18 17:39 ` [PATCH v5 1/2] Documentation: livepatch: Convert to automatically generated contents Mark Brown
2021-01-18 17:39 ` [PATCH v5 2/2] Documentation: livepatch: document reliable stacktrace Mark Brown
2021-01-18 19:25   ` Randy Dunlap [this message]
2021-01-18 18:00 ` [PATCH v5 0/2] Documentation: livepatch: Document reliable stacktrace and minor cleanup Josh Poimboeuf

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=e0243599-f650-9b8e-0ccc-68b6163668f3@infradead.org \
    --to=rdunlap@infradead.org \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-doc@vgert.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.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).