[PATCH v3] x86/unwind/orc: Recheck address range after stack info was updated

Dmitry Monakhov posted 1 patch 4 years ago
arch/x86/kernel/unwind_orc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH v3] x86/unwind/orc: Recheck address range after stack info was updated
Posted by Dmitry Monakhov 4 years ago
Otherwise crash is possible in case of other bug in IBS handling code which
is passing invalid regs to the unwinder.

Original OOPS log:
BUG: stack guard page was hit at 000000000dd984a2 (stack is 00000000d1caafca..00000000613712f0)
kernel stack overflow (page fault): 0000 [#1] SMP NOPTI
CPU: 93 PID: 23787 Comm: context_switch1 Not tainted 5.4.145 #1
RIP: 0010:unwind_next_frame
Call Trace:
 <NMI>
 perf_callchain_kernel
 get_perf_callchain
 perf_callchain
 perf_prepare_sample
 perf_event_output_forward
 __perf_event_overflow
 perf_ibs_handle_irq
 perf_ibs_nmi_handler
 nmi_handle
 default_do_nmi
 do_nmi
 end_repeat_nmi

Signed-off-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
---

Changelog:
 v1->v2: Do not call on_stack() twice for valid range
 v2->v3: Update commit message accrording to Josh's comments

 arch/x86/kernel/unwind_orc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 794fdef2501a..38185aedf7d1 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -339,11 +339,11 @@ static bool stack_access_ok(struct unwind_state *state, unsigned long _addr,
 	struct stack_info *info = &state->stack_info;
 	void *addr = (void *)_addr;
 
-	if (!on_stack(info, addr, len) &&
-	    (get_stack_info(addr, state->task, info, &state->stack_mask)))
-		return false;
+	if (on_stack(info, addr, len))
+		return true;
 
-	return true;
+	return !get_stack_info(addr, state->task, info, &state->stack_mask) &&
+		on_stack(info, addr, len);
 }
 
 static bool deref_stack_reg(struct unwind_state *state, unsigned long addr,
-- 
2.7.4
[tip: x86/urgent] x86/unwind/orc: Recheck address range after stack info was updated
Posted by tip-bot2 for Dmitry Monakhov 4 years ago
The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     6c8ef58a50b5fab6e364b558143490a2014e2a4f
Gitweb:        https://git.kernel.org/tip/6c8ef58a50b5fab6e364b558143490a2014e2a4f
Author:        Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
AuthorDate:    Tue, 19 Apr 2022 10:34:16 +03:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 Apr 2022 21:58:46 +02:00

x86/unwind/orc: Recheck address range after stack info was updated

A crash was observed in the ORC unwinder:

  BUG: stack guard page was hit at 000000000dd984a2 (stack is 00000000d1caafca..00000000613712f0)
  kernel stack overflow (page fault): 0000 [#1] SMP NOPTI
  CPU: 93 PID: 23787 Comm: context_switch1 Not tainted 5.4.145 #1
  RIP: 0010:unwind_next_frame
  Call Trace:
   <NMI>
   perf_callchain_kernel
   get_perf_callchain
   perf_callchain
   perf_prepare_sample
   perf_event_output_forward
   __perf_event_overflow
   perf_ibs_handle_irq
   perf_ibs_nmi_handler
   nmi_handle
   default_do_nmi
   do_nmi
   end_repeat_nmi

This was really two bugs:

  1) The perf IBS code passed inconsistent regs to the unwinder.

  2) The unwinder didn't handle the bad input gracefully.

Fix the latter bug.  The ORC unwinder needs to be immune against bad
inputs.  The problem is that stack_access_ok() doesn't recheck the
validity of the full range of registers after switching to the next
valid stack with get_stack_info().  Fix that.

[ jpoimboe: rewrote commit log ]

Signed-off-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/1650353656-956624-1-git-send-email-dmtrmonakhov@yandex-team.ru
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/kernel/unwind_orc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 794fdef..38185ae 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -339,11 +339,11 @@ static bool stack_access_ok(struct unwind_state *state, unsigned long _addr,
 	struct stack_info *info = &state->stack_info;
 	void *addr = (void *)_addr;
 
-	if (!on_stack(info, addr, len) &&
-	    (get_stack_info(addr, state->task, info, &state->stack_mask)))
-		return false;
+	if (on_stack(info, addr, len))
+		return true;
 
-	return true;
+	return !get_stack_info(addr, state->task, info, &state->stack_mask) &&
+		on_stack(info, addr, len);
 }
 
 static bool deref_stack_reg(struct unwind_state *state, unsigned long addr,