On Mon, Oct 25, 2021 at 05:49:25PM +0100, Mark Rutland wrote:

From f3e66ca75aff3474355839f72d123276028204e1 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 25 Oct 2021 13:23:11 +0100
Subject: [PATCH] arm64: ftrace: use HAVE_FUNCTION_GRAPH_RET_ADDR_PTR

When CONFIG_FUNCTION_GRAPH_TRACER is selected, and the function graph:
tracer is in use, unwind_frame() may erroneously asscociate a traced
function with an incorrect return address. This can happen when starting
an unwind from a pt_regs, or when unwinding across an exception
boundary.

The underlying problem is that ftrace_graph_get_ret_stack() takes an
index offset from the most recent entry added to the fgraph return
stack. We start an unwind at offset 0, and increment the offset each
time we encounter `return_to_handler`, which indicates a rewritten
return address. This is broken in two cases:

* Between creating a pt_regs and starting the unwind, function calls may
  place entries on the stack, leaving an abitrary offset which we can
  only determine by performing a full unwind from the caller of the
  unwind code. While this initial unwind is open-coded in
  dump_backtrace(), this is not performed for other unwinders such as
  perf_callchain_kernel().

* When unwinding across an exception boundary (whether continuing an
  unwind or starting a new unwind from regs), we always consume the LR
  of the interrupted context, though this may not have been live at the
  time of the exception. Where the LR was not live but happened to
  contain `return_to_handler`, we'll recover an address from the graph
  return stack and increment the current offset, leaving subsequent
  entries off-by-one.

  Where the LR was not live and did not contain `return_to_handler`, we
  will still report an erroneous address, but subsequent entries will be
  unaffected.

It turns out I had this backwards, and we currently always *skip* the LR
when unwinding across regs, because:

* The entry assembly creates a synthetic frame record with the original
  FP and the ELR_EL1 value (i.e. the PC at the point of the exception),
  skipping the LR.

* In arch_stack_walk() we start the walk from regs->pc, and continue
  with the frame record, skipping the LR.

* In the existing dump_backtrace, we skip until we hit a frame record
  whose FP value matches the FP in the regs (i.e. the synthetic frame
  record created by the entry assembly). That'll dump the ELR_EL1 value,
  then continue to the next frame record, skipping the LR.

So case two is bogus, and only case one can happen today. This cleanup
shouldn't trigger the WARN_ON_ONCE() in unwind_frame(), and we can fix
the missing LR entry in a subsequent cleanup.

Thanks,
Mark.

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