mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dylan Hatch <dylanbhatch@google.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>,
	Weinan Liu <wnliu@google.com>,  Will Deacon <will@kernel.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	 Indu Bhagat <ibhagatgnu@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	 Steven Rostedt <rostedt@goodmis.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Jiri Kosina <jikos@kernel.org>,
	Jens Remus <jremus@linux.ibm.com>,
	 Prasanna Kumar T S M <ptsm@linux.microsoft.com>,
	Puranjay Mohan <puranjay@kernel.org>,  Song Liu <song@kernel.org>,
	joe.lawrence@redhat.com, linux-toolchains@vger.kernel.org,
	 linux-kernel@vger.kernel.org, live-patching@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v5 8/8] unwind: arm64: Use sframe to unwind interrupt frames
Date: Mon, 11 May 2026 20:00:21 -0700	[thread overview]
Message-ID: <CADBMgpx9YxNUO6wLP7mYKxWW8L78Hk9gPwHrMjXUwPyUmGEu9w@mail.gmail.com> (raw)
In-Reply-To: <afTYzAF_x41pyilu@J2N7QTR9R3>

Hi Mark,

Thanks for all the feedback and help on this. I'm planning on getting
your comments addressed in the coming days, but I have some initial
clarifying questions.

On Fri, May 1, 2026 at 9:46 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Dylan,
>
> Thanks for putting this together. I think this is looking pretty good.
> However, there are some things that aren't quite right and need some
> work, which I've commented on below.
>
> More generally, there are a few things that aren't addressed by this
> series that we will also need to address. Importantly:
>
> (1) For correctness, we'll need to address a latent issue with unwinding
>     across an fgraph return trampoline, where the return address is
>     transiently unrecoverable.
>
>     Before this series, that doesn't matter for livepatching because the
>     livepatching code isn't called synchronously within the fgraph
>     handler, and unwinds which cross an exception boundary are marked as
>     unreliable.
>
>     After this series, that does matter as we can unwind across an
>     exception boundary, and might happen to interrupt that transient
>     window.
>
>     I think we can solve that with some restructuring of that code,
>     restoring the original address *before* removing that from the
>     fgraph return stack, and ensuring that the unwinder can find it.

If my understanding is correct, the issue arrises in return_to_handler
as the return address is recovered:

mov x0, sp
bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs);
mov x30, x0 // restore the original return address

Because ftrace_return_to_handler pops the return address from the
return stack before it can be restored into the LR, it cannot be
recovered.

Based on this, I believe you are suggesting to restructure this code
path such that the return address is removed from the return stack
only after it has been restored to LR. But since kernel/trace/fgraph.c
is core kernel code, will this end up requiring either (1) a similar
restructuring of other arches supporting ftrace, or (2) an
arm64-specific implementation of this recovery logic?

It looks to me like there is essentially the same recovery pattern on
other arches; is there a reason this transient unrecoverability isn't
an issue for reliable unwind on other platforms?

>
>     I'm not immediately sure whether kretprobes has a similar issue.
>
> (2) To make unwinding generally possible, we'll need to annotate some
>     assembly functions as unwindable. We'll need to do that for string
>     routines under lib/, and probably some crypto code, but we don't
>     need to do that for most code in head.S, entry.S, etc.
>
>     The vast majority of relevant assembly functions are leaf functions
>     (where the return address is never moved out of the LR), so we can
>     probably get away with a simple annotation for those that avoids the
>     need for open-coded CFI directives everywhere.

Are you suggesting something like a SYM_LEAF_FUNC_(START|END), that
wraps CFI directives for leaf functions?

>
> I've pushed some reliable stacktrace tests to:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git stacktrace/tests
>
> That finds the fgraph issue (regardless of this series). When merged
> with this series triggers a warning in kunwind_next_frame_record_meta(),
> where unwind_next_frame_sframe() calls that erroneously as a fallback.

Thanks for the pointer on these tests, they're super useful! I've been
able to reproduce the fgraph failure you mentioned.

Thanks,
Dylan

  parent reply	other threads:[~2026-05-12  3:00 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-28 18:36 [PATCH v5 0/8] unwind, arm64: add sframe unwinder for kernel Dylan Hatch
2026-04-28 18:36 ` [PATCH v5 1/8] sframe: Allow kernelspace sframe sections Dylan Hatch
2026-04-28 18:36 ` [PATCH v5 2/8] arm64, unwind: build kernel with sframe V3 info Dylan Hatch
2026-04-28 18:36 ` [PATCH v5 3/8] arm64: entry: add unwind info for various kernel entries Dylan Hatch
2026-04-29 15:26   ` Mark Rutland
2026-05-15  3:30     ` Dylan Hatch
2026-05-15  8:58       ` Mark Rutland
2026-05-18 22:41         ` Dylan Hatch
2026-04-28 18:36 ` [PATCH v5 4/8] sframe: Provide PC lookup for vmlinux .sframe section Dylan Hatch
2026-04-28 18:36 ` [PATCH v5 5/8] sframe: Allow unsorted FDEs Dylan Hatch
2026-04-30 10:04   ` Jens Remus
2026-04-28 18:36 ` [PATCH v5 6/8] arm64/module, sframe: Add sframe support for modules Dylan Hatch
2026-04-30 10:04   ` Jens Remus
2026-04-28 18:36 ` [PATCH v5 7/8] sframe: Introduce in-kernel SFRAME_VALIDATION Dylan Hatch
2026-04-30 10:04   ` Jens Remus
2026-04-28 18:36 ` [PATCH v5 8/8] unwind: arm64: Use sframe to unwind interrupt frames Dylan Hatch
2026-05-01 16:46   ` Mark Rutland
2026-05-04  8:47     ` Jens Remus
2026-05-05 10:29       ` Mark Rutland
2026-05-05 15:52         ` Jens Remus
2026-05-12  3:00     ` Dylan Hatch [this message]
2026-05-12  8:55       ` Jens Remus
2026-05-12 10:18         ` Mark Rutland
2026-05-12 10:07       ` Mark Rutland
2026-05-19  6:29     ` Dylan Hatch
2026-04-29 17:18 ` [PATCH v5 0/8] unwind, arm64: add sframe unwinder for kernel Mark Rutland
2026-04-30 10:11 ` Jens Remus
2026-05-12  1:10   ` Dylan Hatch
2026-05-15 11:32 ` Mostafa Saleh
2026-05-18 17:55   ` Dylan Hatch
2026-05-19 11:05     ` Jens Remus
2026-07-14  0:28       ` Brian Norris
2026-07-15  8:06         ` Jens Remus
2026-07-15 16:20           ` Brian Norris
2026-07-16 11:03             ` Jens Remus
2026-07-16 17:39               ` Brian Norris

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=CADBMgpx9YxNUO6wLP7mYKxWW8L78Hk9gPwHrMjXUwPyUmGEu9w@mail.gmail.com \
    --to=dylanbhatch@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=ibhagatgnu@gmail.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=jremus@linux.ibm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=ptsm@linux.microsoft.com \
    --cc=puranjay@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=song@kernel.org \
    --cc=will@kernel.org \
    --cc=wnliu@google.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