Index | Thread | Search

From:
Christian Ludwig <cludwig@genua.de>
Subject:
Maintain preempted copy(9) state in stacktrace_save_utrace()
To:
<tech@openbsd.org>
Date:
Wed, 12 Mar 2025 21:42:58 +0100

Download raw body.

Thread
Hi,

A btrace(8) probe can run in interrupt context which preempts copy(9).
Care must be taken getting userland stacktraces in that situation.
stacktrace_save_utrace() uses copyin() itself, so we need to save and
restore the preempted context's copy(9) state.

This approach is less invasive than the previous one
(https://marc.info/?l=openbsd-tech&m=174136347113129).

Tested on amd64 and i386.


 - Christian

---
 sys/arch/amd64/amd64/db_trace.c | 7 +++++++
 sys/arch/i386/i386/db_trace.c   | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/sys/arch/amd64/amd64/db_trace.c b/sys/arch/amd64/amd64/db_trace.c
index 1e7fcf2208ad..fe5ff40fa50b 100644
--- a/sys/arch/amd64/amd64/db_trace.c
+++ b/sys/arch/amd64/amd64/db_trace.c
@@ -292,6 +292,7 @@ stacktrace_save_utrace(struct stacktrace *st)
 {
 	struct callframe f, *frame, *lastframe;
 	struct pcb *pcb = curpcb;
+	caddr_t onfault;
 
 	st->st_count = 0;
 
@@ -302,7 +303,11 @@ stacktrace_save_utrace(struct stacktrace *st)
 	KASSERT(INKERNEL(frame));
 	f = *frame;
 
+	/* CAREFUL: Might be in interrupt context and/or in copy(9). */
 	curcpu()->ci_inatomic++;
+	onfault = pcb->pcb_onfault;
+	pcb->pcb_onfault = NULL;
+
 	while (st->st_count < STACKTRACE_MAX) {
 		if (f.f_retaddr != 0 && !INKERNEL(f.f_retaddr))
 			st->st_pc[st->st_count++] = f.f_retaddr;
@@ -323,6 +328,8 @@ stacktrace_save_utrace(struct stacktrace *st)
 		if (copyin(frame, &f, sizeof(f)) != 0)
 			break;
 	}
+
+	pcb->pcb_onfault = onfault;
 	curcpu()->ci_inatomic--;
 }
 
diff --git a/sys/arch/i386/i386/db_trace.c b/sys/arch/i386/i386/db_trace.c
index 2e1c9faefca7..4e413dc19245 100644
--- a/sys/arch/i386/i386/db_trace.c
+++ b/sys/arch/i386/i386/db_trace.c
@@ -291,6 +291,7 @@ stacktrace_save_utrace(struct stacktrace *st)
 {
 	struct callframe f, *frame, *lastframe;
 	struct pcb *pcb = curpcb;
+	caddr_t onfault;
 
 	st->st_count = 0;
 
@@ -301,7 +302,11 @@ stacktrace_save_utrace(struct stacktrace *st)
 	KASSERT(INKERNEL(frame));
 	f = *frame;
 
+	/* CAREFUL: Might be in interrupt context and/or in copy(9). */
 	curcpu()->ci_inatomic++;
+	onfault = pcb->pcb_onfault;
+	pcb->pcb_onfault = NULL;
+
 	while (st->st_count < STACKTRACE_MAX) {
 		if (f.f_retaddr != 0 && !INKERNEL(f.f_retaddr))
 			st->st_pc[st->st_count++] = f.f_retaddr;
@@ -322,6 +327,8 @@ stacktrace_save_utrace(struct stacktrace *st)
 		if (copyin(frame, &f, sizeof(f)) != 0)
 			break;
 	}
+
+	pcb->pcb_onfault = onfault;
 	curcpu()->ci_inatomic--;
 }
 
-- 
2.34.1