From 20a0d5cf0bdbf40da21dfad70d7a8ecab6bdaef8 Mon Sep 17 00:00:00 2001
From: visa <visa@openbsd.org>
Date: Sun, 29 Mar 2020 15:14:28 +0000
Subject: [PATCH] Prevent stack trace saving from inspecting untrusted data. On
 amd64, arm64 and i386, the chain of call frames is continuous from kernel to
 userspace. The unwinder has to stop at the latest when it reaches the start
 of the kernel stack.

OK mpi@
---
 sys/arch/amd64/amd64/db_trace.c | 18 +++++++++++++-----
 sys/arch/arm64/arm64/db_trace.c | 23 ++++++++++++++++++-----
 sys/arch/i386/i386/db_trace.c   | 18 +++++++++++++-----
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/sys/arch/amd64/amd64/db_trace.c b/sys/arch/amd64/amd64/db_trace.c
index 3d4bdce456e..c26c386cb4b 100644
--- a/sys/arch/amd64/amd64/db_trace.c
+++ b/sys/arch/amd64/amd64/db_trace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_trace.c,v 1.50 2020/03/25 14:59:23 mpi Exp $	*/
+/*	$OpenBSD: db_trace.c,v 1.51 2020/03/29 15:14:28 visa Exp $	*/
 /*	$NetBSD: db_trace.c,v 1.1 2003/04/26 18:39:27 fvdl Exp $	*/
 
 /*
@@ -258,10 +258,18 @@ db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
 void
 stacktrace_save_at(struct stacktrace *st, unsigned int skip)
 {
-	struct callframe *frame, *lastframe;
+	struct callframe *frame, *lastframe, *limit;
+	struct pcb *pcb = curpcb;
+
+	st->st_count = 0;
+
+	if (pcb == NULL)
+		return;
 
 	frame = __builtin_frame_address(0);
-	st->st_count = 0;
+	KASSERT(INKERNEL(frame));
+	limit = (struct callframe *)((struct trapframe *)pcb->pcb_kstack - 1);
+
 	while (st->st_count < STACKTRACE_MAX) {
 		if (skip == 0)
 			st->st_pc[st->st_count++] = frame->f_retaddr;
@@ -271,10 +279,10 @@ stacktrace_save_at(struct stacktrace *st, unsigned int skip)
 		lastframe = frame;
 		frame = frame->f_frame;
 
-		if (!INKERNEL(frame))
-			break;
 		if (frame <= lastframe)
 			break;
+		if (frame >= limit)
+			break;
 		if (!INKERNEL(frame->f_retaddr))
 			break;
 	}
diff --git a/sys/arch/arm64/arm64/db_trace.c b/sys/arch/arm64/arm64/db_trace.c
index 46c49fcfa52..8ea1b463e56 100644
--- a/sys/arch/arm64/arm64/db_trace.c
+++ b/sys/arch/arm64/arm64/db_trace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_trace.c,v 1.9 2020/01/20 15:58:23 visa Exp $	*/
+/*	$OpenBSD: db_trace.c,v 1.10 2020/03/29 15:14:28 visa Exp $	*/
 /*	$NetBSD: db_trace.c,v 1.8 2003/01/17 22:28:48 thorpej Exp $	*/
 
 /*
@@ -152,16 +152,29 @@ db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
 void
 stacktrace_save(struct stacktrace *st)
 {
-	struct callframe *frame;
+	struct callframe *frame, *lastframe, *limit;
+	struct proc *p = curproc;
+
+	st->st_count = 0;
+
+	if (p == NULL)
+		return;
 
 	frame = __builtin_frame_address(0);
-	st->st_count = 0;
+	KASSERT(INKERNEL(frame));
+	limit = (struct callframe *)STACKALIGN(p->p_addr + USPACE -
+	    sizeof(struct trapframe) - 0x10);
+
 	while (st->st_count < STACKTRACE_MAX) {
 		st->st_pc[st->st_count++] = frame->f_lr;
 
-		if (!INKERNEL(frame->f_frame) || frame->f_frame <= frame)
-			break;
+		lastframe = frame;
 		frame = frame->f_frame;
+
+		if (frame <= lastframe)
+			break;
+		if (frame >= limit)
+			break;
 		if (!INKERNEL(frame->f_lr))
 			break;
 	}
diff --git a/sys/arch/i386/i386/db_trace.c b/sys/arch/i386/i386/db_trace.c
index 97aed9b6097..60787cf2395 100644
--- a/sys/arch/i386/i386/db_trace.c
+++ b/sys/arch/i386/i386/db_trace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_trace.c,v 1.39 2020/03/25 14:59:23 mpi Exp $	*/
+/*	$OpenBSD: db_trace.c,v 1.40 2020/03/29 15:14:28 visa Exp $	*/
 /*	$NetBSD: db_trace.c,v 1.18 1996/05/03 19:42:01 christos Exp $	*/
 
 /*
@@ -263,10 +263,18 @@ db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
 void
 stacktrace_save_at(struct stacktrace *st, unsigned int skip)
 {
-	struct callframe *frame, *lastframe;
+	struct callframe *frame, *lastframe, *limit;
+	struct pcb *pcb = curpcb;
+
+	st->st_count = 0;
+
+	if (pcb == NULL)
+		return;
 
 	frame = __builtin_frame_address(0);
-	st->st_count = 0;
+	KASSERT(INKERNEL(frame));
+	limit = (struct callframe *)((struct trapframe *)pcb->pcb_kstack - 1);
+
 	while (st->st_count < STACKTRACE_MAX) {
 		if (skip == 0)
 			st->st_pc[st->st_count++] = frame->f_retaddr;
@@ -276,10 +284,10 @@ stacktrace_save_at(struct stacktrace *st, unsigned int skip)
 		lastframe = frame;
 		frame = frame->f_frame;
 
-		if (!INKERNEL(frame))
-			break;
 		if (frame <= lastframe)
 			break;
+		if (frame >= limit)
+			break;
 		if (!INKERNEL(frame->f_retaddr))
 			break;
 	}