Date:   Mon, 18 Jan 2021 11:25:03 -0800
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org
Cc:     Mark Rutland <mark.rutland@....com>,
        Jiri Kosina <jikos@...nel.org>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Miroslav Benes <mbenes@...e.cz>,
        Petr Mladek <pmladek@...e.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        linux-doc@...rt.kernel.org, live-patching@...r.kernel.org
Subject: Re: [PATCH v5 2/2] Documentation: livepatch: document reliable
 stacktrace

Hi,

On 1/18/21 9:39 AM, Mark Brown wrote:
> From: Mark Rutland <mark.rutland@....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@....com>
> [Updates following review -- broonie]
> Acked-by: Josh Poimboeuf <jpoimboe@...hat.com>
> Signed-off-by: Mark Brown <broonie@...nel.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@...radead.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

Powered by blists - more mailing lists