Hello,

I've hit a kernel crash when I was playing with btrace(8).
The traced program is openssl application which is linked
with openssl library. The library itself is compiled with
functions which are implemented in assembly. Those functions
seem to corrupt a stack frame when they are entered.

The kernel crashes here at line 317:

311
312                 if (frame == NULL)
313                         break;
314                 if (INKERNEL(f.f_retaddr)) {
315                         if (frame <= lastframe)
316                                 break;
317                         f = *frame;
318                         continue;
319                 }
320                 if (!INKERNEL(lastframe) && frame <= lastframe)
321                         break;
322                 if (copyin(frame, &f, sizeof(f)) != 0)
323                         break;

at line 317 we may be reading memory which comes from userland
via copyin at line 322 (the code in question runs in while loop).
the f.f_retaddr might be corrupted/invalid.

The proposed fix below makes sure we treat all frames as userland
frames as soon as we call copyin() for the first time.

I was able to trigger panic on i386, but I think amd64 suffers
from the same issue.

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 1e7fcf2208a..17a0d442382 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;
+	int userland = 0;
 
 	st->st_count = 0;
 
@@ -312,7 +313,7 @@ stacktrace_save_utrace(struct stacktrace *st)
 
 		if (frame == NULL)
 			break;
-		if (INKERNEL(f.f_retaddr)) {
+		if (userland == 0 && INKERNEL(f.f_retaddr)) {
 			if (frame <= lastframe)
 				break;
 			f = *frame;
@@ -322,6 +323,7 @@ stacktrace_save_utrace(struct stacktrace *st)
 			break;
 		if (copyin(frame, &f, sizeof(f)) != 0)
 			break;
+		userland = 1;
 	}
 	curcpu()->ci_inatomic--;
 }
diff --git a/sys/arch/i386/i386/db_trace.c b/sys/arch/i386/i386/db_trace.c
index 2e1c9faefca..3a851743088 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;
+	int userland = 0;
 
 	st->st_count = 0;
 
@@ -311,7 +312,10 @@ stacktrace_save_utrace(struct stacktrace *st)
 
 		if (frame == NULL)
 			break;
-		if (INKERNEL(f.f_retaddr)) {
+		/*
+		 * Don't trust f.f_retaddr if it came from userland via copyin
+		 */
+		if (userland == 0 && INKERNEL(f.f_retaddr)) {
 			if (frame <= lastframe)
 				break;
 			f = *frame;
@@ -321,6 +325,7 @@ stacktrace_save_utrace(struct stacktrace *st)
 			break;
 		if (copyin(frame, &f, sizeof(f)) != 0)
 			break;
+		userland = 1;
 	}
 	curcpu()->ci_inatomic--;
 }