git: 1be56e0bb1e8 - main - arm/unwind: Check stack pointer boundaries before dereferencing

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Thu, 27 Jul 2023 20:12:55 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=1be56e0bb1e8bd8373e446ff9386bcdd764935aa

commit 1be56e0bb1e8bd8373e446ff9386bcdd764935aa
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-07-27 19:44:00 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-07-27 19:44:00 +0000

    arm/unwind: Check stack pointer boundaries before dereferencing
    
    If the unwinder somehow ends up with a stack pointer that lies outside
    the stack, then an attempt to dereference can lead to a fault, which
    causes the kernel to panic again and unwind the stack, which leads to a
    fault...
    
    Add kstack_contains() checks at points where we dereference the stack
    pointer.  This avoids the aforementioned infinite loop in one case I hit
    where some OpenSSL assembly code apparently confuses the unwinder.
    
    Reviewed by:    jhb
    MFC after:      2 weeks
    Sponsored by:   Klara, Inc.
    Sponsored by:   Stormshield
    Differential Revision:  https://reviews.freebsd.org/D41210
---
 sys/arm/arm/unwind.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sys/arm/arm/unwind.c b/sys/arm/arm/unwind.c
index bf8ffddfd2c2..5d3309d4539d 100644
--- a/sys/arm/arm/unwind.c
+++ b/sys/arm/arm/unwind.c
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/kernel.h>
 #include <sys/linker.h>
 #include <sys/malloc.h>
+#include <sys/proc.h>
 #include <sys/queue.h>
 #include <sys/systm.h>
 
@@ -370,6 +371,7 @@ unwind_exec_read_byte(struct unwind_state *state)
 static int
 unwind_exec_insn(struct unwind_state *state)
 {
+	struct thread *td = curthread;
 	unsigned int insn;
 	uint32_t *vsp = (uint32_t *)state->registers[SP];
 	int update_vsp = 0;
@@ -404,6 +406,10 @@ unwind_exec_insn(struct unwind_state *state)
 		/* Load the registers */
 		for (reg = 4; mask && reg < 16; mask >>= 1, reg++) {
 			if (mask & 1) {
+				if (!kstack_contains(td, (uintptr_t)vsp,
+				    sizeof(*vsp)))
+					return 1;
+
 				state->registers[reg] = *vsp++;
 				state->update_mask |= 1 << reg;
 
@@ -430,6 +436,9 @@ unwind_exec_insn(struct unwind_state *state)
 		update_vsp = 1;
 
 		/* Pop the registers */
+		if (!kstack_contains(td, (uintptr_t)vsp,
+		    sizeof(*vsp) * (4 + count)))
+			return 1;
 		for (reg = 4; reg <= 4 + count; reg++) {
 			state->registers[reg] = *vsp++;
 			state->update_mask |= 1 << reg;
@@ -437,6 +446,8 @@ unwind_exec_insn(struct unwind_state *state)
 
 		/* Check if we are in the pop r14 version */
 		if ((insn & INSN_POP_TYPE_MASK) != 0) {
+			if (!kstack_contains(td, (uintptr_t)vsp, sizeof(*vsp)))
+				return 1;
 			state->registers[14] = *vsp++;
 		}
 
@@ -457,6 +468,9 @@ unwind_exec_insn(struct unwind_state *state)
 		/* Load the registers */
 		for (reg = 0; mask && reg < 4; mask >>= 1, reg++) {
 			if (mask & 1) {
+				if (!kstack_contains(td, (uintptr_t)vsp,
+				    sizeof(*vsp)))
+					return 1;
 				state->registers[reg] = *vsp++;
 				state->update_mask |= 1 << reg;
 			}