linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	broonie@kernel.org, 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,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH v2 0/4] arm64: Implement stack trace reliability checks
Date: Fri, 9 Apr 2021 17:53:21 -0500	[thread overview]
Message-ID: <20210409225321.2czbawz6p2aquf5m@treble> (raw)
In-Reply-To: <20210409223227.rvf6tfhvgnpzmabn@treble>

On Fri, Apr 09, 2021 at 05:32:27PM -0500, Josh Poimboeuf wrote:
> On Fri, Apr 09, 2021 at 05:05:58PM -0500, Madhavan T. Venkataraman wrote:
> > > FWIW, over the years we've had zero issues with encoding the frame
> > > pointer on x86.  After you save pt_regs, you encode the frame pointer to
> > > point to it.  Ideally in the same macro so it's hard to overlook.
> > > 
> > 
> > I had the same opinion. In fact, in my encoding scheme, I have additional
> > checks to make absolutely sure that it is a true encoding and not stack
> > corruption. The chances of all of those values accidentally matching are,
> > well, null.
> 
> Right, stack corruption -- which is already exceedingly rare -- would
> have to be combined with a miracle or two in order to come out of the
> whole thing marked as 'reliable' :-)
> 
> And really, we already take a similar risk today by "trusting" the frame
> pointer value on the stack to a certain extent.

Oh yeah, I forgot to mention some more benefits of encoding the frame
pointer (or marking pt_regs in some other way):

a) Stack addresses can be printed properly: '%pS' for printing regs->pc
   and '%pB' for printing call returns.

   Using '%pS' for call returns (as arm64 seems to do today) will result
   in printing the wrong function when you have tail calls to noreturn
   functions on the stack (which is actually quite common for calls to
   panic(), die(), etc).

   More details:

   https://lkml.kernel.org/r/20210403155948.ubbgtwmlsdyar7yp@treble

b) Stack dumps to the console can dump the exception registers they find
   along the way.  This is actually quite nice for debugging.


-- 
Josh


  reply	other threads:[~2021-04-09 22:53 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <705993ccb34a611c75cdae0a8cb1b40f9b218ebd>
2021-04-05 20:43 ` [RFC PATCH v2 0/4] arm64: Implement stack trace reliability checks madvenka
2021-04-05 20:43   ` [RFC PATCH v2 1/4] arm64: Implement infrastructure for " madvenka
2021-04-08 15:15     ` Mark Brown
2021-04-08 17:17     ` Mark Brown
2021-04-08 19:30       ` Madhavan T. Venkataraman
2021-04-08 23:30         ` Madhavan T. Venkataraman
2021-04-09 11:57           ` Mark Brown
2021-04-05 20:43   ` [RFC PATCH v2 2/4] arm64: Mark a stack trace unreliable if an EL1 exception frame is detected madvenka
2021-04-05 20:43   ` [RFC PATCH v2 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable madvenka
2021-04-08 16:58     ` Mark Brown
2021-04-08 19:23       ` Madhavan T. Venkataraman
2021-04-09 11:31         ` Mark Brown
2021-04-09 14:02           ` Madhavan T. Venkataraman
2021-04-09 12:27     ` Mark Rutland
2021-04-09 17:23       ` Madhavan T. Venkataraman
2021-04-05 20:43   ` [RFC PATCH v2 4/4] arm64: Mark stack trace as unreliable if kretprobed functions are present madvenka
2021-04-09 12:09   ` [RFC PATCH v2 0/4] arm64: Implement stack trace reliability checks Mark Rutland
2021-04-09 17:16     ` Madhavan T. Venkataraman
2021-04-09 21:37     ` Josh Poimboeuf
2021-04-09 22:05       ` Madhavan T. Venkataraman
2021-04-09 22:32         ` Josh Poimboeuf
2021-04-09 22:53           ` Josh Poimboeuf [this message]
2021-04-11 17:54             ` Madhavan T. Venkataraman
2021-04-12 16:59           ` Mark Brown
2021-04-13 22:53             ` Josh Poimboeuf
2021-04-14 12:24               ` Mark Brown
2021-04-12 17:36       ` Mark Brown
2021-04-12 19:55         ` Madhavan T. Venkataraman
2021-04-13 11:02           ` Mark Brown
2021-04-14 10:23             ` Madhavan T. Venkataraman
2021-04-14 12:35               ` Mark Brown
2021-04-16 14:43               ` Madhavan T. Venkataraman
2021-04-16 15:36                 ` Mark Brown

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=20210409225321.2czbawz6p2aquf5m@treble \
    --to=jpoimboe@redhat.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.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=madvenka@linux.microsoft.com \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=will@kernel.org \
    /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).