[Devel] [PATCH RHEL7 COMMIT] ms/x86/unwind: Disable KASAN checks for non-current tasks

Konstantin Khorenko khorenko at virtuozzo.com
Wed Aug 30 14:12:22 MSK 2023


The commit is pushed to "branch-rh7-3.10.0-1160.95.1.vz7.210.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.95.1.vz7.210.2
------>
commit 6d89da59786450975b3dafd5977423bd4898cc6b
Author: Josh Poimboeuf <jpoimboe at redhat.com>
Date:   Mon Jan 9 12:00:23 2017 -0600

    ms/x86/unwind: Disable KASAN checks for non-current tasks
    
    There are a handful of callers to save_stack_trace_tsk() and
    show_stack() which try to unwind the stack of a task other than current.
    In such cases, it's remotely possible that the task is running on one
    CPU while the unwinder is reading its stack from another CPU, causing
    the unwinder to see stack corruption.
    
    These cases seem to be mostly harmless.  The unwinder has checks which
    prevent it from following bad pointers beyond the bounds of the stack.
    So it's not really a bug as long as the caller understands that
    unwinding another task will not always succeed.
    
    In such cases, it's possible that the unwinder may read a KASAN-poisoned
    region of the stack.  Account for that by using READ_ONCE_NOCHECK() when
    reading the stack of another task.
    
    Use READ_ONCE() when reading the stack of the current task, since KASAN
    warnings can still be useful for finding bugs in that case.
    
    Reported-by: Dmitry Vyukov <dvyukov at google.com>
    Signed-off-by: Josh Poimboeuf <jpoimboe at redhat.com>
    Cc: Andy Lutomirski <luto at amacapital.net>
    Cc: Andy Lutomirski <luto at kernel.org>
    Cc: Borislav Petkov <bp at alien8.de>
    Cc: Brian Gerst <brgerst at gmail.com>
    Cc: Dave Jones <davej at codemonkey.org.uk>
    Cc: Denys Vlasenko <dvlasenk at redhat.com>
    Cc: H. Peter Anvin <hpa at zytor.com>
    Cc: Linus Torvalds <torvalds at linux-foundation.org>
    Cc: Miroslav Benes <mbenes at suse.cz>
    Cc: Peter Zijlstra <peterz at infradead.org>
    Cc: Thomas Gleixner <tglx at linutronix.de>
    Link: http://lkml.kernel.org/r/4c575eb288ba9f73d498dfe0acde2f58674598f1.1483978430.git.jpoimboe@redhat.com
    Signed-off-by: Ingo Molnar <mingo at kernel.org>
    
    (cherry picked from ms commit 84936118bdf37bda513d4a361c38181a216427e0)
    in the scope of https://jira.sw.ru/browse/HCI-171
    and
    in the scope of https://jira.vzint.dev/browse/PSBM-149932
    
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 arch/x86/include/asm/stacktrace.h |  5 ++++-
 arch/x86/kernel/unwind_frame.c    | 19 +++++++++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index f8a383338352..71714402d0af 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -52,6 +52,8 @@ static inline bool on_stack(struct stack_info *info, void *addr, size_t len)
 static inline unsigned long *
 get_frame_pointer(struct task_struct *task, struct pt_regs *regs)
 {
+	unsigned long *frame;
+
 	if (regs)
 		return (unsigned long *)regs->bp;
 
@@ -59,7 +61,8 @@ get_frame_pointer(struct task_struct *task, struct pt_regs *regs)
 		return __builtin_frame_address(0);
 
 	/* bp is the last reg pushed by switch_to */
-	return (unsigned long *)*(unsigned long *)task->thread.sp;
+	frame = (unsigned long *)task->thread.sp;
+	return (unsigned long *)READ_ONCE_NOCHECK(*frame);
 }
 #else
 static inline unsigned long *
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 05a9f845c417..0a7d7d265053 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -6,6 +6,21 @@
 
 #define FRAME_HEADER_SIZE (sizeof(long) * 2)
 
+/*
+ * This disables KASAN checking when reading a value from another task's stack,
+ * since the other task could be running on another CPU and could have poisoned
+ * the stack in the meantime.
+ */
+#define READ_ONCE_TASK_STACK(task, x)			\
+({							\
+	unsigned long val;				\
+	if (task == current)				\
+		val = READ_ONCE(x);			\
+	else						\
+		val = READ_ONCE_NOCHECK(x);		\
+	val;						\
+})
+
 unsigned long unwind_get_return_address(struct unwind_state *state)
 {
 	if (unwind_done(state))
@@ -44,7 +59,7 @@ static bool update_stack_state(struct unwind_state *state,
 	/* Move state to the next frame: */
 	state->bp = next_bp;
 	addr_p = state->bp + 1;
-	addr = READ_ONCE(*addr_p);
+	addr = READ_ONCE_TASK_STACK(state->task, *addr_p);
 	state->ip = ftrace_graph_ret_addr(state->task, &state->graph_idx,
 					  addr, addr_p);
 
@@ -113,7 +128,7 @@ bool unwind_next_frame(struct unwind_state *state)
 	if (unwind_end(state))
 		goto the_end;
 
-	next_bp = (unsigned long *)READ_ONCE(*state->bp);
+	next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task,*state->bp);
 
 	/* make sure the next frame's data is accessible */
 	if (!update_stack_state(state, next_bp)) {


More information about the Devel mailing list