diff mbox series

[1/2] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2

Message ID 9583327904ebbbeda399eca9c56d6c7085ac20fe.1612534649.git.jpoimboe@redhat.com
State Accepted
Commit e504e74cc3a2c092b05577ce3e8e013fae7d94e6
Headers show
Series [1/2] x86/unwind/orc: Disable KASAN checking in the ORC unwinder, part 2 | expand

Commit Message

Josh Poimboeuf Feb. 5, 2021, 2:24 p.m. UTC
KASAN reserves "redzone" areas between stack frames in order to detect
stack overruns.  A read or write to such an area triggers a KASAN
"stack-out-of-bounds" BUG.

Normally, the ORC unwinder stays in-bounds and doesn't access the
redzone.  But sometimes it can't find ORC metadata for a given
instruction.  This can happen for code which is missing ORC metadata, or
for generated code.  In such cases, the unwinder attempts to fall back
to frame pointers, as a best-effort type thing.

This fallback often works, but when it doesn't, the unwinder can get
confused and go off into the weeds into the KASAN redzone, triggering
the aforementioned KASAN BUG.

But in this case, the unwinder's confusion is actually harmless and
working as designed.  It already has checks in place to prevent
off-stack accesses, but those checks get short-circuited by the KASAN
BUG.  And a BUG is a lot more disruptive than a harmless unwinder
warning.

Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
accesses.  This finishes the job started by commit 881125bfe65b
("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
partially fixed the issue.

Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Cc: stable@vger.kernel.org
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/unwind_orc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Steven Rostedt Feb. 5, 2021, 2:51 p.m. UTC | #1
On Fri,  5 Feb 2021 08:24:02 -0600
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> KASAN reserves "redzone" areas between stack frames in order to detect
> stack overruns.  A read or write to such an area triggers a KASAN
> "stack-out-of-bounds" BUG.
> 
> Normally, the ORC unwinder stays in-bounds and doesn't access the
> redzone.  But sometimes it can't find ORC metadata for a given
> instruction.  This can happen for code which is missing ORC metadata, or
> for generated code.  In such cases, the unwinder attempts to fall back
> to frame pointers, as a best-effort type thing.
> 
> This fallback often works, but when it doesn't, the unwinder can get
> confused and go off into the weeds into the KASAN redzone, triggering
> the aforementioned KASAN BUG.
> 
> But in this case, the unwinder's confusion is actually harmless and
> working as designed.  It already has checks in place to prevent
> off-stack accesses, but those checks get short-circuited by the KASAN
> BUG.  And a BUG is a lot more disruptive than a harmless unwinder
> warning.
> 
> Disable the KASAN checks by using READ_ONCE_NOCHECK() for all stack
> accesses.  This finishes the job started by commit 881125bfe65b
> ("x86/unwind: Disable KASAN checking in the ORC unwinder"), which only
> partially fixed the issue.
> 
> Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
> Reported-by: Ivan Babrou <ivan@cloudflare.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve
diff mbox series

Patch

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 73f800100066..c451d5f6422f 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -367,8 +367,8 @@  static bool deref_stack_regs(struct unwind_state *state, unsigned long addr,
 	if (!stack_access_ok(state, addr, sizeof(struct pt_regs)))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -380,8 +380,8 @@  static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr
 	if (!stack_access_ok(state, addr, IRET_FRAME_SIZE))
 		return false;
 
-	*ip = regs->ip;
-	*sp = regs->sp;
+	*ip = READ_ONCE_NOCHECK(regs->ip);
+	*sp = READ_ONCE_NOCHECK(regs->sp);
 	return true;
 }
 
@@ -402,12 +402,12 @@  static bool get_reg(struct unwind_state *state, unsigned int reg_off,
 		return false;
 
 	if (state->full_regs) {
-		*val = ((unsigned long *)state->regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->regs)[reg]);
 		return true;
 	}
 
 	if (state->prev_regs) {
-		*val = ((unsigned long *)state->prev_regs)[reg];
+		*val = READ_ONCE_NOCHECK(((unsigned long *)state->prev_regs)[reg]);
 		return true;
 	}