5132 mdb loops forever printing a stack backtrace

Reviewed by: Richard Lowe <richlowe@richlowe.net>
Reviewed by: Adam Leventhal <ahl@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>

4 files changed, 49 insertions, 19 deletions

diff --git a/usr/src/cmd/mdb/common/mdb/mdb_err.c b/usr/src/cmd/mdb/common/mdb/mdb_err.c
index 4c31551cde..beddad2aa2 100644
--- a/usr/src/cmd/mdb/common/mdb/mdb_err.c
+++ b/usr/src/cmd/mdb/common/mdb/mdb_err.c
@@ -24,8 +24,6 @@
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <mdb/mdb_signal.h>
 #include <mdb/mdb_err.h>
 #include <mdb/mdb.h>
@@ -114,7 +112,9 @@ static const char *const _mdb_errlist[] = {
 	"boot-loaded kmdb cannot be unloaded",		/* EMDB_KNOUNLOAD */
 	"too many enabled watchpoints for this machine", /* EMDB_WPTOOMANY */
 	"DTrace is active",				/* EMDB_DTACTIVE */
-	"boot-loaded module cannot be unloaded"		/* EMDB_KMODNOUNLOAD */
+	"boot-loaded module cannot be unloaded",	/* EMDB_KMODNOUNLOAD */
+	"stack frame pointer is invalid"		/* EMDB_STKFRAME */
+
 };
 
 static const int _mdb_nerr = sizeof (_mdb_errlist) / sizeof (_mdb_errlist[0]);
diff --git a/usr/src/cmd/mdb/common/mdb/mdb_errno.h b/usr/src/cmd/mdb/common/mdb/mdb_errno.h
index 09dd877e82..7dfaff2896 100644
--- a/usr/src/cmd/mdb/common/mdb/mdb_errno.h
+++ b/usr/src/cmd/mdb/common/mdb/mdb_errno.h
@@ -27,8 +27,6 @@
 #ifndef _MDB_ERRNO_H
 #define	_MDB_ERRNO_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -114,7 +112,8 @@ enum {
 	EMDB_KNOUNLOAD,				/* kmdb cannot be unloaded */
 	EMDB_WPTOOMANY,				/* Too many watchpoints */
 	EMDB_DTACTIVE,				/* DTrace is active */
-	EMDB_KMODNOUNLOAD			/* module can't be unloaded */
+	EMDB_KMODNOUNLOAD,			/* module can't be unloaded */
+	EMDB_STKFRAME				/* Bad stack frame pointer */
 };
 
 #endif /* _MDB */
diff --git a/usr/src/cmd/mdb/intel/mdb/mdb_amd64util.c b/usr/src/cmd/mdb/intel/mdb/mdb_amd64util.c
index 432f22210c..b56a7388e3 100644
--- a/usr/src/cmd/mdb/intel/mdb/mdb_amd64util.c
+++ b/usr/src/cmd/mdb/intel/mdb/mdb_amd64util.c
@@ -24,8 +24,8 @@
  * Use is subject to license terms.
  */
 /*
- * Copyright 2013 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2012, Joyent, Inc.  All rights reserved.
+ * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/types.h>
@@ -220,6 +220,7 @@ mdb_amd64_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 	uint_t argc, reg_argc;
 	long fr_argv[32];
 	int start_index; /* index to save_instr where to start comparison */
+	int err;
 	int i;
 
 	struct {
@@ -229,7 +230,7 @@ mdb_amd64_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 
 	uintptr_t fp = gsp->kregs[KREG_RBP];
 	uintptr_t pc = gsp->kregs[KREG_RIP];
-	uintptr_t lastfp;
+	uintptr_t lastfp = 0;
 
 	ssize_t size;
 	ssize_t insnsize;
@@ -251,11 +252,22 @@ mdb_amd64_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 	while (fp != 0) {
 		int args_style = 0;
 
-		if (!fp_is_aligned(fp, xpv_panic))
-			return (set_errno(EMDB_STKALIGN));
-
-		if (mdb_tgt_vread(t, &fr, sizeof (fr), fp) != sizeof (fr))
-			return (-1);	/* errno has been set for us */
+		/*
+		 * Ensure progress (increasing fp), and prevent
+		 * endless loop with the same FP.
+		 */
+		if (fp <= lastfp) {
+			err = EMDB_STKFRAME;
+			goto badfp;
+		}
+		if (!fp_is_aligned(fp, xpv_panic)) {
+			err = EMDB_STKALIGN;
+			goto badfp;
+		}
+		if (mdb_tgt_vread(t, &fr, sizeof (fr), fp) != sizeof (fr)) {
+			err = EMDB_NOMAP;
+			goto badfp;
+		}
 
 		if ((mdb_tgt_lookup_by_addr(t, pc, MDB_TGT_SYM_FUZZY,
 		    NULL, 0, &s, &sip) == 0) &&
@@ -384,6 +396,10 @@ mdb_amd64_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 	}
 
 	return (0);
+
+badfp:
+	mdb_printf("%p [%s]", fp, mdb_strerror(err));
+	return (set_errno(err));
 }
 
 /*
diff --git a/usr/src/cmd/mdb/intel/mdb/mdb_ia32util.c b/usr/src/cmd/mdb/intel/mdb/mdb_ia32util.c
index 43db94741b..80ce1c7ad2 100644
--- a/usr/src/cmd/mdb/intel/mdb/mdb_ia32util.c
+++ b/usr/src/cmd/mdb/intel/mdb/mdb_ia32util.c
@@ -24,6 +24,7 @@
  */
 /*
  * Copyright (c) 2012, Joyent, Inc.  All rights reserved.
+ * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/types.h>
@@ -194,6 +195,7 @@ mdb_ia32_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 	mdb_tgt_gregset_t gregs;
 	kreg_t *kregs = &gregs.kregs[0];
 	int got_pc = (gsp->kregs[KREG_EIP] != 0);
+	int err;
 
 	struct {
 		uintptr_t fr_savfp;
@@ -203,7 +205,7 @@ mdb_ia32_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 
 	uintptr_t fp = gsp->kregs[KREG_EBP];
 	uintptr_t pc = gsp->kregs[KREG_EIP];
-	uintptr_t lastfp;
+	uintptr_t lastfp = 0;
 
 	ssize_t size;
 	uint_t argc;
@@ -219,16 +221,25 @@ mdb_ia32_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 
 	while (fp != 0) {
 
-		if (fp & (STACK_ALIGN - 1))
-			return (set_errno(EMDB_STKALIGN));
-
+		/*
+		 * Ensure progress (increasing fp), and prevent
+		 * endless loop with the same FP.
+		 */
+		if (fp <= lastfp) {
+			err = EMDB_STKFRAME;
+			goto badfp;
+		}
+		if (fp & (STACK_ALIGN - 1)) {
+			err = EMDB_STKALIGN;
+			goto badfp;
+		}
 		if ((size = mdb_tgt_vread(t, &fr, sizeof (fr), fp)) >=
 		    (ssize_t)(2 * sizeof (uintptr_t))) {
 			size -= (ssize_t)(2 * sizeof (uintptr_t));
 			argc = kvm_argcount(t, fr.fr_savpc, size);
 		} else {
-			bzero(&fr, sizeof (fr));
-			argc = 0;
+			err = EMDB_NOMAP;
+			goto badfp;
 		}
 
 		if (got_pc && func(arg, pc, argc, fr.fr_argv, &gregs) != 0)
@@ -257,6 +268,10 @@ mdb_ia32_kvm_stack_iter(mdb_tgt_t *t, const mdb_tgt_gregset_t *gsp,
 	}
 
 	return (0);
+
+badfp:
+	mdb_printf("%p [%s]", fp, mdb_strerror(err));
+	return (set_errno(err));
 }
 
 /*