All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Miroslav Benes <mbenes@suse.cz>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>,
	Dave Jones <dsj@fb.com>, Jann Horn <jannh@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vince Weaver <vincent.weaver@maine.edu>
Subject: [PATCH 4.19 43/48] x86/unwind/orc: Fix premature unwind stoppage due to IRET frames
Date: Wed, 13 May 2020 11:45:09 +0200	[thread overview]
Message-ID: <20200513094403.458754310@linuxfoundation.org> (raw)
In-Reply-To: <20200513094351.100352960@linuxfoundation.org>

From: Josh Poimboeuf <jpoimboe@redhat.com>

commit 81b67439d147677d844d492fcbd03712ea438f42 upstream.

The following execution path is possible:

  fsnotify()
    [ realign the stack and store previous SP in R10 ]
    <IRQ>
      [ only IRET regs saved ]
      common_interrupt()
        interrupt_entry()
	  <NMI>
	    [ full pt_regs saved ]
	    ...
	    [ unwind stack ]

When the unwinder goes through the NMI and the IRQ on the stack, and
then sees fsnotify(), it doesn't have access to the value of R10,
because it only has the five IRET registers.  So the unwind stops
prematurely.

However, because the interrupt_entry() code is careful not to clobber
R10 before saving the full regs, the unwinder should be able to read R10
from the previously saved full pt_regs associated with the NMI.

Handle this case properly.  When encountering an IRET regs frame
immediately after a full pt_regs frame, use the pt_regs as a backup
which can be used to get the C register values.

Also, note that a call frame resets the 'prev_regs' value, because a
function is free to clobber the registers.  For this fix to work, the
IRET and full regs frames must be adjacent, with no FUNC frames in
between.  So replace the FUNC hint in interrupt_entry() with an
IRET_REGS hint.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Jones <dsj@fb.com>
Cc: Jann Horn <jannh@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: https://lore.kernel.org/r/97a408167cc09f1cfa0de31a7b70dd88868d743f.1587808742.git.jpoimboe@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/x86/entry/entry_64.S     |    4 +--
 arch/x86/include/asm/unwind.h |    2 -
 arch/x86/kernel/unwind_orc.c  |   51 ++++++++++++++++++++++++++++++++----------
 3 files changed, 43 insertions(+), 14 deletions(-)

--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -575,7 +575,7 @@ END(spurious_entries_start)
  * +----------------------------------------------------+
  */
 ENTRY(interrupt_entry)
-	UNWIND_HINT_FUNC
+	UNWIND_HINT_IRET_REGS offset=16
 	ASM_CLAC
 	cld
 
@@ -607,9 +607,9 @@ ENTRY(interrupt_entry)
 	pushq	5*8(%rdi)		/* regs->eflags */
 	pushq	4*8(%rdi)		/* regs->cs */
 	pushq	3*8(%rdi)		/* regs->ip */
+	UNWIND_HINT_IRET_REGS
 	pushq	2*8(%rdi)		/* regs->orig_ax */
 	pushq	8(%rdi)			/* return address */
-	UNWIND_HINT_FUNC
 
 	movq	(%rdi), %rdi
 	jmp	2f
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -19,7 +19,7 @@ struct unwind_state {
 #if defined(CONFIG_UNWINDER_ORC)
 	bool signal, full_regs;
 	unsigned long sp, bp, ip;
-	struct pt_regs *regs;
+	struct pt_regs *regs, *prev_regs;
 #elif defined(CONFIG_UNWINDER_FRAME_POINTER)
 	bool got_irq;
 	unsigned long *bp, *orig_sp, ip;
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -364,9 +364,38 @@ static bool deref_stack_iret_regs(struct
 	return true;
 }
 
+/*
+ * If state->regs is non-NULL, and points to a full pt_regs, just get the reg
+ * value from state->regs.
+ *
+ * Otherwise, if state->regs just points to IRET regs, and the previous frame
+ * had full regs, it's safe to get the value from the previous regs.  This can
+ * happen when early/late IRQ entry code gets interrupted by an NMI.
+ */
+static bool get_reg(struct unwind_state *state, unsigned int reg_off,
+		    unsigned long *val)
+{
+	unsigned int reg = reg_off/8;
+
+	if (!state->regs)
+		return false;
+
+	if (state->full_regs) {
+		*val = ((unsigned long *)state->regs)[reg];
+		return true;
+	}
+
+	if (state->prev_regs) {
+		*val = ((unsigned long *)state->prev_regs)[reg];
+		return true;
+	}
+
+	return false;
+}
+
 bool unwind_next_frame(struct unwind_state *state)
 {
-	unsigned long ip_p, sp, orig_ip = state->ip, prev_sp = state->sp;
+	unsigned long ip_p, sp, tmp, orig_ip = state->ip, prev_sp = state->sp;
 	enum stack_type prev_type = state->stack_info.type;
 	struct orc_entry *orc;
 	bool indirect = false;
@@ -420,39 +449,35 @@ bool unwind_next_frame(struct unwind_sta
 		break;
 
 	case ORC_REG_R10:
-		if (!state->regs || !state->full_regs) {
+		if (!get_reg(state, offsetof(struct pt_regs, r10), &sp)) {
 			orc_warn("missing regs for base reg R10 at ip %pB\n",
 				 (void *)state->ip);
 			goto err;
 		}
-		sp = state->regs->r10;
 		break;
 
 	case ORC_REG_R13:
-		if (!state->regs || !state->full_regs) {
+		if (!get_reg(state, offsetof(struct pt_regs, r13), &sp)) {
 			orc_warn("missing regs for base reg R13 at ip %pB\n",
 				 (void *)state->ip);
 			goto err;
 		}
-		sp = state->regs->r13;
 		break;
 
 	case ORC_REG_DI:
-		if (!state->regs || !state->full_regs) {
+		if (!get_reg(state, offsetof(struct pt_regs, di), &sp)) {
 			orc_warn("missing regs for base reg DI at ip %pB\n",
 				 (void *)state->ip);
 			goto err;
 		}
-		sp = state->regs->di;
 		break;
 
 	case ORC_REG_DX:
-		if (!state->regs || !state->full_regs) {
+		if (!get_reg(state, offsetof(struct pt_regs, dx), &sp)) {
 			orc_warn("missing regs for base reg DX at ip %pB\n",
 				 (void *)state->ip);
 			goto err;
 		}
-		sp = state->regs->dx;
 		break;
 
 	default:
@@ -479,6 +504,7 @@ bool unwind_next_frame(struct unwind_sta
 
 		state->sp = sp;
 		state->regs = NULL;
+		state->prev_regs = NULL;
 		state->signal = false;
 		break;
 
@@ -490,6 +516,7 @@ bool unwind_next_frame(struct unwind_sta
 		}
 
 		state->regs = (struct pt_regs *)sp;
+		state->prev_regs = NULL;
 		state->full_regs = true;
 		state->signal = true;
 		break;
@@ -501,6 +528,8 @@ bool unwind_next_frame(struct unwind_sta
 			goto err;
 		}
 
+		if (state->full_regs)
+			state->prev_regs = state->regs;
 		state->regs = (void *)sp - IRET_FRAME_OFFSET;
 		state->full_regs = false;
 		state->signal = true;
@@ -515,8 +544,8 @@ bool unwind_next_frame(struct unwind_sta
 	/* Find BP: */
 	switch (orc->bp_reg) {
 	case ORC_REG_UNDEFINED:
-		if (state->regs && state->full_regs)
-			state->bp = state->regs->bp;
+		if (get_reg(state, offsetof(struct pt_regs, bp), &tmp))
+			state->bp = tmp;
 		break;
 
 	case ORC_REG_PREV_SP:



  parent reply	other threads:[~2020-05-13  9:47 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13  9:44 [PATCH 4.19 00/48] 4.19.123-rc1 review Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 01/48] USB: serial: qcserial: Add DW5816e support Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 02/48] tracing/kprobes: Fix a double initialization typo Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 03/48] vt: fix unicode console freeing with a common interface Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 04/48] dp83640: reverse arguments to list_add_tail Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 05/48] fq_codel: fix TCA_FQ_CODEL_DROP_BATCH_SIZE sanity checks Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 06/48] net: macsec: preserve ingress frame ordering Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 07/48] net/mlx4_core: Fix use of ENOSPC around mlx4_counter_alloc() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 08/48] net_sched: sch_skbprio: add message validation to skbprio_change() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 09/48] net: usb: qmi_wwan: add support for DW5816e Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 10/48] sch_choke: avoid potential panic in choke_reset() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 11/48] sch_sfq: validate silly quantum values Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 12/48] tipc: fix partial topology connection closure Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 13/48] bnxt_en: Fix VLAN acceleration handling in bnxt_fix_features() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 14/48] net/mlx5: Fix forced completion access non initialized command entry Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 15/48] net/mlx5: Fix command entry leak in Internal Error State Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 16/48] bnxt_en: Improve AER slot reset Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 17/48] bnxt_en: Fix VF anti-spoof filter setup Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 18/48] net: stricter validation of untrusted gso packets Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 19/48] HID: wacom: Read HID_DG_CONTACTMAX directly for non-generic devices Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 20/48] sctp: Fix bundling of SHUTDOWN with COOKIE-ACK Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 21/48] HID: usbhid: Fix race between usbhid_close() and usbhid_stop() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 22/48] USB: uas: add quirk for LaCie 2Big Quadra Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 23/48] USB: serial: garmin_gps: add sanity checking for data length Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 24/48] tracing: Add a vmalloc_sync_mappings() for safe measure Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 25/48] KVM: arm: vgic: Fix limit condition when writing to GICD_I[CS]ACTIVER Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 26/48] KVM: arm64: Fix 32bit PC wrap-around Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 27/48] arm64: hugetlb: avoid potential NULL dereference Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 28/48] mm/page_alloc: fix watchdog soft lockups during set_zone_contiguous() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 29/48] staging: gasket: Check the return value of gasket_get_bar_index() Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 30/48] coredump: fix crash when umh is disabled Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 31/48] KVM: VMX: Explicitly reference RCX as the vmx_vcpu pointer in asm blobs Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 32/48] KVM: VMX: Mark RCX, RDX and RSI as clobbered in vmx_vcpu_run()s asm blob Greg Kroah-Hartman
2020-05-13  9:44 ` [PATCH 4.19 33/48] batman-adv: fix batadv_nc_random_weight_tq Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 34/48] batman-adv: Fix refcnt leak in batadv_show_throughput_override Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 35/48] batman-adv: Fix refcnt leak in batadv_store_throughput_override Greg Kroah-Hartman
2020-05-13 21:03   ` Pavel Machek
2020-05-13  9:45 ` [PATCH 4.19 36/48] batman-adv: Fix refcnt leak in batadv_v_ogm_process Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 37/48] x86/entry/64: Fix unwind hints in register clearing code Greg Kroah-Hartman
2020-05-13 21:48   ` Pavel Machek
2020-05-14 19:27     ` Josh Poimboeuf
2020-05-13  9:45 ` [PATCH 4.19 38/48] x86/entry/64: Fix unwind hints in kernel exit path Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 39/48] x86/entry/64: Fix unwind hints in rewind_stack_do_exit() Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 40/48] x86/unwind/orc: Dont skip the first frame for inactive tasks Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 41/48] x86/unwind/orc: Prevent unwinding before ORC initialization Greg Kroah-Hartman
2020-05-13 21:52   ` Pavel Machek
2020-05-14 19:44     ` Josh Poimboeuf
2020-05-14 20:13       ` Pavel Machek
2020-05-14 20:28         ` Josh Poimboeuf
2020-05-13  9:45 ` [PATCH 4.19 42/48] x86/unwind/orc: Fix error path for bad ORC entry type Greg Kroah-Hartman
2020-05-13  9:45 ` Greg Kroah-Hartman [this message]
2020-05-13  9:45 ` [PATCH 4.19 44/48] netfilter: nat: never update the UDP checksum when its 0 Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 45/48] netfilter: nf_osf: avoid passing pointer to local var Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 46/48] objtool: Fix stack offset tracking for indirect CFAs Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 47/48] scripts/decodecode: fix trapping instruction formatting Greg Kroah-Hartman
2020-05-13  9:45 ` [PATCH 4.19 48/48] ipc/mqueue.c: change __do_notify() to bypass check_kill_permission() Greg Kroah-Hartman
     [not found] ` <20200513094351.100352960-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
2020-05-13 13:45   ` [PATCH 4.19 00/48] 4.19.123-rc1 review Jon Hunter
2020-05-13 13:45     ` Jon Hunter
2020-05-13 17:03 ` Guenter Roeck
2020-05-13 18:14 ` Naresh Kamboju
2020-05-13 19:29 ` Chris Paterson
2020-05-13 23:02 ` shuah

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=20200513094403.458754310@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dsj@fb.com \
    --cc=jannh@google.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mbenes@suse.cz \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.weaver@maine.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.