linux-kernel - [PATCH v2 2/5] x86/stacktrace: remove unwind

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue,  5 Dec 2017 09:55:35 +0100
From:   Jiri Slaby <jslaby@...e.cz>
To:     mingo@...hat.com
Cc:     live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jiri Slaby <jslaby@...e.cz>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: [PATCH v2 2/5] x86/stacktrace: remove unwind_state->error

Reorganize the unwinding in __save_stack_trace_reliable, so that we
don't need to set another variable -- unwind_state->error. In case,
unwinding fails, we fail after the for loop too. The only way to escape
the loop successfully is via the 'if (user_mode(regs))' check now.

Signed-off-by: Jiri Slaby <jslaby@...e.cz>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: "H. Peter Anvin" <hpa@...or.com>
Cc: x86@...nel.org
Cc: Josh Poimboeuf <jpoimboe@...hat.com>
---
 arch/x86/include/asm/unwind.h  |  6 ------
 arch/x86/kernel/stacktrace.c   | 15 ++++++---------
 arch/x86/kernel/unwind_frame.c |  2 --
 3 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 5be2fb23825a..6f90295e6547 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -15,7 +15,6 @@ struct unwind_state {
 	unsigned long stack_mask;
 	struct task_struct *task;
 	int graph_idx;
-	bool error;
 #if defined(CONFIG_UNWINDER_ORC)
 	bool signal, full_regs;
 	unsigned long sp, bp, ip;
@@ -40,11 +39,6 @@ static inline bool unwind_done(struct unwind_state *state)
 	return state->stack_info.type == STACK_TYPE_UNKNOWN;
 }
 
-static inline bool unwind_error(struct unwind_state *state)
-{
-	return state->error;
-}
-
 static inline
 void unwind_start(struct unwind_state *state, struct task_struct *task,
 		  struct pt_regs *regs, unsigned long *first_frame)
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index b088d4f9e43e..9193607e3ead 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -104,16 +104,16 @@ __save_stack_trace_reliable(struct stack_trace *trace,
 
 		regs = unwind_get_entry_regs(&state);
 		if (regs) {
+			if (user_mode(regs))
+				goto success;
+
 			/*
 			 * Kernel mode registers on the stack indicate an
 			 * in-kernel interrupt or exception (e.g., preemption
 			 * or a page fault), which can make frame pointers
 			 * unreliable.
 			 */
-			if (!user_mode(regs))
-				return -EINVAL;
-
-			break;
+			return -EINVAL;
 		}
 
 		addr = unwind_get_return_address(&state);
@@ -132,12 +132,9 @@ __save_stack_trace_reliable(struct stack_trace *trace,
 			return -EINVAL;
 	}
 
-	/* Check for stack corruption */
-	if (unwind_error(&state)) {
-		STACKTRACE_DUMP_ONCE(task);
-		return -EINVAL;
-	}
+	return -EINVAL;
 
+success:
 	if (trace->nr_entries < trace->max_entries)
 		trace->entries[trace->nr_entries++] = ULONG_MAX;
 
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 3dc26f95d46e..0c08a56adabe 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -332,8 +332,6 @@ bool unwind_next_frame(struct unwind_state *state)
 	return true;
 
 bad_address:
-	state->error = true;
-
 	/*
 	 * When unwinding a non-current task, the task might actually be
 	 * running on another CPU, in which case it could be modifying its
-- 
2.15.1