linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Sasha Levin <sashal@kernel.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	linuxppc-dev@lists.ozlabs.org, Nicolai Stange <nstange@suse.de>
Subject: [PATCH AUTOSEL 5.0 202/262] powerpc/64s: Clear on-stack exception marker upon exception return
Date: Wed, 27 Mar 2019 14:00:57 -0400	[thread overview]
Message-ID: <20190327180158.10245-202-sashal@kernel.org> (raw)
In-Reply-To: <20190327180158.10245-1-sashal@kernel.org>

From: Nicolai Stange <nstange@suse.de>

[ Upstream commit eddd0b332304d554ad6243942f87c2fcea98c56b ]

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.

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>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/powerpc/kernel/entry_64.S | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 435927f549c4..a2c168b395d2 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	ld	r2,_NIP(r1)
 	mtspr	SPRN_SRR0,r2
 
+	/*
+	 * Leaving a stale exception_marker on the stack can confuse
+	 * the reliable stack unwinder later on. Clear it.
+	 */
+	li	r2,0
+	std	r2,STACK_FRAME_OVERHEAD-16(r1)
+
 	ld	r0,GPR0(r1)
 	ld	r2,GPR2(r1)
 	ld	r3,GPR3(r1)
-- 
2.19.1


  parent reply	other threads:[~2019-03-27 18:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190327180158.10245-1-sashal@kernel.org>
2019-03-27 17:57 ` [PATCH AUTOSEL 5.0 015/262] memblock: memblock_phys_alloc_try_nid(): don't panic Sasha Levin
2019-03-28  5:57   ` Mike Rapoport
2019-04-04  0:57     ` Sasha Levin
2019-03-27 17:58 ` [PATCH AUTOSEL 5.0 061/262] mm/resource: Return real error codes from walk failures Sasha Levin
2019-03-27 17:58 ` [PATCH AUTOSEL 5.0 072/262] powerpc/powernv/ioda: Fix locked_vm counting for memory used by IOMMU tables Sasha Levin
2019-03-27 17:58 ` [PATCH AUTOSEL 5.0 076/262] powerpc/fsl: Fix the flush of branch predictor Sasha Levin
2019-03-27 17:58 ` [PATCH AUTOSEL 5.0 080/262] powerpc/xmon: Fix opcode being uninitialized in print_insn_powerpc Sasha Levin
2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 086/262] powerpc/hugetlb: Handle mmap_min_addr correctly in get_unmapped_area callback Sasha Levin
2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 106/262] powerpc/44x: Force PCI on for CURRITUCK Sasha Levin
2019-03-27 17:59 ` [PATCH AUTOSEL 5.0 118/262] SoC: imx-sgtl5000: add missing put_device() Sasha Levin
2019-03-27 18:00 ` [PATCH AUTOSEL 5.0 174/262] powerpc/ptrace: Mitigate potential Spectre v1 Sasha Levin
2019-03-27 18:00 ` [PATCH AUTOSEL 5.0 183/262] ASoC: fsl-asoc-card: fix object reference leaks in fsl_asoc_card_probe Sasha Levin
2019-03-27 18:00 ` Sasha Levin [this message]
2019-03-27 18:01 ` [PATCH AUTOSEL 5.0 206/262] powerpc/pseries: Perform full re-add of CPU for topology update post-migration Sasha Levin

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=20190327180158.10245-202-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nstange@suse.de \
    --cc=stable@vger.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).