linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <patch-notifications@ellerman.id.au>
To: Joe Lawrence <joe.lawrence@redhat.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	live-patching@vger.kernel.org
Cc: Jiri Kosina <jkosina@suse.cz>, Torsten Duwe <duwe@lst.de>,
	Nicolai Stange <nstange@suse.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [1/4] powerpc/64s: Clear on-stack exception marker upon exception return
Date: Sat,  9 Feb 2019 00:02:33 +1100 (AEDT)	[thread overview]
Message-ID: <43wwMG0vPgz9sNL@ozlabs.org> (raw)
In-Reply-To: <20190122155724.27557-2-joe.lawrence@redhat.com>

On Tue, 2019-01-22 at 15:57:21 UTC, Joe Lawrence wrote:
> From: Nicolai Stange <nstange@suse.de>
> 
> The ppc64 specific implementation of the reliable stacktracer,
> save_stack_trace_tsk_reliable(), bails out and reports an "unreliable
> trace" whenever it finds an exception frame on the stack. Stack frames
> are classified as exception frames if the STACK_FRAME_REGS_MARKER magic,
> as written by exception prologues, is found at a particular location.
> 
> However, as observed by Joe Lawrence, it is possible in practice that
> non-exception stack frames can alias with prior exception frames and thus,
> that the reliable stacktracer can find a stale STACK_FRAME_REGS_MARKER on
> the stack. It in turn falsely reports an unreliable stacktrace and blocks
> any live patching transition to finish. Said condition lasts until the
> stack frame is overwritten/initialized by function call or other means.
> 
> In principle, we could mitigate this by making the exception frame
> classification condition in save_stack_trace_tsk_reliable() stronger:
> in addition to testing for STACK_FRAME_REGS_MARKER, we could also take into
> account that for all exceptions executing on the kernel stack
> - their stack frames's backlink pointers always match what is saved
>   in their pt_regs instance's ->gpr[1] slot and that
> - their exception frame size equals STACK_INT_FRAME_SIZE, a value
>   uncommonly large for non-exception frames.
> 
> However, while these are currently true, relying on them would make the
> reliable stacktrace implementation more sensitive towards future changes in
> the exception entry code. Note that false negatives, i.e. not detecting
> exception frames, would silently break the live patching consistency model.
> 
> Furthermore, certain other places (diagnostic stacktraces, perf, xmon)
> rely on STACK_FRAME_REGS_MARKER as well.
> 
> Make the exception exit code clear the on-stack STACK_FRAME_REGS_MARKER
> for those exceptions running on the "normal" kernel stack and returning
> to kernelspace: because the topmost frame is ignored by the reliable stack
> tracer anyway, returns to userspace don't need to take care of clearing
> the marker.
> 
> Furthermore, as I don't have the ability to test this on Book 3E or
> 32 bits, limit the change to Book 3S and 64 bits.
> 
> Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on
> PPC_BOOK3S_64 for documentation purposes. Before this patch, it depended
> on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies
> PPC_BOOK3S_64, there's no functional change here.
> 
> Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for the consistency model")
> Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/eddd0b332304d554ad6243942f87c2fc

cheers

  parent reply	other threads:[~2019-02-08 14:04 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 15:57 [PATCH 0/4] powerpc/livepatch: reliable stack unwinder fixes Joe Lawrence
2019-01-22 15:57 ` [PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return Joe Lawrence
2019-01-30 12:27   ` Michael Ellerman
2019-01-30 17:18     ` Nicolai Stange
2019-01-31  5:46       ` Michael Ellerman
2019-02-02  1:14   ` Balbir Singh
2019-02-02  3:42     ` Balbir Singh
2019-02-05 11:24       ` Michael Ellerman
2019-02-06  2:48         ` Balbir Singh
2019-02-06  4:44           ` Michael Ellerman
2019-02-06  8:45             ` Balbir Singh
2019-02-08 13:02   ` Michael Ellerman [this message]
2019-01-22 15:57 ` [PATCH 2/4] powerpc/livepatch: relax reliable stack tracer checks for first-frame Joe Lawrence
2019-01-22 15:57 ` [PATCH 3/4] powerpc/livepatch: small cleanups in save_stack_trace_tsk_reliable() Joe Lawrence
2019-01-22 15:57 ` [PATCH 4/4] powerpc/livepatch: return -ERRNO values " Joe Lawrence
2019-02-02  0:59   ` Balbir Singh
2019-01-29 21:10 ` [PATCH 0/4] powerpc/livepatch: reliable stack unwinder fixes Josh Poimboeuf
2019-01-29 23:50 ` Jiri Kosina
2019-01-30 12:22   ` Michael Ellerman
2019-01-30 13:28     ` Jiri Kosina
2019-01-31  5:46       ` Michael Ellerman

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=43wwMG0vPgz9sNL@ozlabs.org \
    --to=patch-notifications@ellerman.id.au \
    --cc=duwe@lst.de \
    --cc=jkosina@suse.cz \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=live-patching@vger.kernel.org \
    --cc=nstange@suse.de \
    /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).