From: Alexandr Nedvedicky <sashan@fastmail.net>
Subject: follow up on 'stacktrace_save_utrace() should be more robust'
To: tech@openbsd.org
Cc: tedu@tedunangst.com
Date: Sun, 8 Jun 2025 15:10:52 +0200

Hello,

this is a finishing touch to change which got committed two weeks ago [1].
The current code copies invalid stack frame. This tweak just discards it.

OK to commit?

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------

diff --git a/sys/arch/amd64/amd64/db_trace.c b/sys/arch/amd64/amd64/db_trace.c
index 624d7ed1a73..79dcf745a21 100644
--- a/sys/arch/amd64/amd64/db_trace.c
+++ b/sys/arch/amd64/amd64/db_trace.c
@@ -318,8 +318,14 @@ stacktrace_save_utrace(struct stacktrace *st)
 		st->st_pc[st->st_count++] = lastframe->f_retaddr;
 
 	while (frame != NULL && st->st_count < STACKTRACE_MAX) {
-		if (copyin(frame, &f, sizeof(f)) != 0)
+		if (copyin(frame, &f, sizeof(f)) != 0) {
+			/*
+			 * the last return address (if saved) is invalid,
+			 * strip it
+			 */
+			st->st_count -= (st->st_count == 0) ? 0 : 1;
 			break;
+		}
 		st->st_pc[st->st_count++] = f.f_retaddr;
 		frame = f.f_frame;
 	}
diff --git a/sys/arch/i386/i386/db_trace.c b/sys/arch/i386/i386/db_trace.c
index b7c8b68711a..6d988dc4d57 100644
--- a/sys/arch/i386/i386/db_trace.c
+++ b/sys/arch/i386/i386/db_trace.c
@@ -317,8 +317,14 @@ stacktrace_save_utrace(struct stacktrace *st)
 		st->st_pc[st->st_count++] = lastframe->f_retaddr;
 
 	while (frame != NULL && st->st_count < STACKTRACE_MAX) {
-		if (copyin(frame, &f, sizeof(f)) != 0)
+		if (copyin(frame, &f, sizeof(f)) != 0) {
+			/*
+			 * the last return address (if saved) is invalid,
+			 * strip it
+			 */
+			st->st_count -= (st->st_count == 0) ? 0 : 1;
 			break;
+		}
 		st->st_pc[st->st_count++] = f.f_retaddr;
 		frame = f.f_frame;
 	}
[1] https://marc.info/?l=openbsd-cvs&m=174796965116589&w=2