changeset 2922:42a733b126fb

6478044 dtrace assert: kaddr >= kernelbase && kaddr + size >= kaddr in dtrace_copyin() 6479887 DTRACE_INRANGE incorrectly treats testsz as a signed quantity 6479991 some err.* test cases need work
author dp
date Fri, 13 Oct 2006 18:01:44 -0700
parents 43242a22930d
children da82ab368162
files usr/src/cmd/dtrace/test/pkg/SUNWdtrt/prototype_com usr/src/cmd/dtrace/test/tst/common/funcs/err.badalloca.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badalloca2.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy1.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy2.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy3.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy4.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy5.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy6.d usr/src/cmd/dtrace/test/tst/common/funcs/err.badchill.d usr/src/cmd/dtrace/test/tst/common/funcs/err.copyout.d usr/src/cmd/dtrace/test/tst/common/safety/tst.copyin2.d usr/src/uts/common/dtrace/dtrace.c
diffstat 14 files changed, 177 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/cmd/dtrace/test/pkg/SUNWdtrt/prototype_com	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/pkg/SUNWdtrt/prototype_com	Fri Oct 13 18:01:44 2006 -0700
@@ -493,6 +493,7 @@
 f none opt/SUNWdtrt/tst/common/funcs/err.D_STRINGOF_TYPE.badstringof.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/funcs/err.D_VAR_UNDEF.badvar.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/funcs/err.badalloca.d 0444 root bin
+f none opt/SUNWdtrt/tst/common/funcs/err.badalloca2.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/funcs/err.badbcopy.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/funcs/err.badbcopy1.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/funcs/err.badbcopy2.d 0444 root bin
@@ -963,6 +964,7 @@
 f none opt/SUNWdtrt/tst/common/safety/tst.caller.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/safety/tst.cleanpath.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/safety/tst.copyin.d 0444 root bin
+f none opt/SUNWdtrt/tst/common/safety/tst.copyin2.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/safety/tst.ddi_pathname.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/safety/tst.dirname.d 0444 root bin
 f none opt/SUNWdtrt/tst/common/safety/tst.execname.d 0444 root bin
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badalloca.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badalloca.d	Fri Oct 13 18:01:44 2006 -0700
@@ -47,6 +47,7 @@
 tick-1
 {
 	bcopy((void *)&`kmem_flags, ptr, sizeof (int));
+	exit(0);
 }
 
 ERROR
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badalloca2.d	Fri Oct 13 18:01:44 2006 -0700
@@ -0,0 +1,49 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Use is subject to license terms.
+ */
+
+#pragma	ident	"%Z%%M%	%I%	%E% SMI"
+
+
+/*
+ * ASSERTION:
+ *	alloca() cannot be used to "unconsume" scratch space memory by
+ *	accepting a negative offset.
+ *
+ * SECTION: Actions and Subroutines/alloca()
+ *
+ */
+
+BEGIN
+{
+	ptr = alloca(10);
+	ptr = alloca(0xffffffffffffffff);
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy.d	Fri Oct 13 18:01:44 2006 -0700
@@ -46,7 +46,7 @@
 	/* Attempt a copy from scratch memory to a kernel address */
 
 	bcopy(ptr, (void *)&`kmem_flags, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy1.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy1.d	Fri Oct 13 18:01:44 2006 -0700
@@ -45,7 +45,7 @@
 
 	/* Attempt to bcopy to scratch memory that isn't allocated */
 	bcopy((void *)&`kmem_flags, ptr, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy2.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy2.d	Fri Oct 13 18:01:44 2006 -0700
@@ -44,7 +44,7 @@
 	/* Attempt to copy to non-scratch memory */
 
 	bcopy((void *)&`kmem_flags, ptr, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy3.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy3.d	Fri Oct 13 18:01:44 2006 -0700
@@ -41,7 +41,7 @@
 {
 	/* Attempt to copy to a NULL address */
 	bcopy((void *)&`kmem_flags, (void *)NULL, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy4.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy4.d	Fri Oct 13 18:01:44 2006 -0700
@@ -44,7 +44,7 @@
 
 	/* Attempt to copy from a NULL address */
 	bcopy((void *)NULL, ptr, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy5.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy5.d	Fri Oct 13 18:01:44 2006 -0700
@@ -46,7 +46,7 @@
 
 	/* Attempt to copy from a invalid address */
 	bcopy(badptr, ptr, sizeof (int));
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy6.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badbcopy6.d	Fri Oct 13 18:01:44 2006 -0700
@@ -31,7 +31,7 @@
 BEGIN
 {
 	bcopy("bad news", alloca(1), -1);
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.badchill.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.badchill.d	Fri Oct 13 18:01:44 2006 -0700
@@ -30,7 +30,7 @@
 {
 	chill(200);
 	chill(((hrtime_t)1 << 63) - 1);
-	exit(1);
+	exit(0);
 }
 
 ERROR
--- a/usr/src/cmd/dtrace/test/tst/common/funcs/err.copyout.d	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/cmd/dtrace/test/tst/common/funcs/err.copyout.d	Fri Oct 13 18:01:44 2006 -0700
@@ -34,6 +34,7 @@
  *
  */
 
+#pragma D option destructive
 
 BEGIN
 {
@@ -42,3 +43,7 @@
 	exit(0);
 }
 
+ERROR
+{
+	exit(1);
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/usr/src/cmd/dtrace/test/tst/common/safety/tst.copyin2.d	Fri Oct 13 18:01:44 2006 -0700
@@ -0,0 +1,67 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Use is subject to license terms.
+ */
+
+#pragma ident	"%Z%%M%	%I%	%E% SMI"
+
+/*
+ * Test that there is no value of 'size' which can be passed to copyin
+ * to cause mischief.  The somewhat odd order of operations ensures
+ * that we test both size = 0 and size = 0xfff...fff
+ */
+#include <sys/types.h>
+
+
+#if defined(_LP64)
+#define MAX_BITS 63
+size_t size;
+#else
+#define MAX_BITS 31
+size_t size;
+#endif
+
+syscall:::
+/pid == $pid/
+{
+	printf("size = 0x%lx\n", (ulong_t)size);
+}
+
+syscall:::
+/pid == $pid/
+{
+	tracemem(copyin(curthread->t_procp->p_user.u_envp, size), 10);
+}
+
+syscall:::
+/pid == $pid && size > (1 << MAX_BITS)/
+{
+	exit(0);
+}
+
+syscall:::
+/pid == $pid/
+{
+	size = (size << 1ULL) | 1ULL;
+}
--- a/usr/src/uts/common/dtrace/dtrace.c	Fri Oct 13 14:23:20 2006 -0700
+++ b/usr/src/uts/common/dtrace/dtrace.c	Fri Oct 13 18:01:44 2006 -0700
@@ -354,12 +354,25 @@
 
 /*
  * Test whether a range of memory starting at testaddr of size testsz falls
- * within the range of memory described by addr, sz, taking care to avoid
- * problems with overflow and underflow of the unsigned quantities.
+ * within the range of memory described by addr, sz.  We take care to avoid
+ * problems with overflow and underflow of the unsigned quantities, and
+ * disallow all negative sizes.  Ranges of size 0 are allowed.
  */
 #define	DTRACE_INRANGE(testaddr, testsz, baseaddr, basesz) \
 	((testaddr) - (baseaddr) < (basesz) && \
-	(testaddr) + (testsz) - (baseaddr) <= (basesz))
+	(testaddr) + (testsz) - (baseaddr) <= (basesz) && \
+	(testaddr) + (testsz) >= (testaddr))
+
+/*
+ * Test whether alloc_sz bytes will fit in the scratch region.  We isolate
+ * alloc_sz on the righthand side of the comparison in order to avoid overflow
+ * or underflow in the comparison with it.  This is simpler than the INRANGE
+ * check above, because we know that the dtms_scratch_ptr is valid in the
+ * range.  Allocations of size zero are allowed.
+ */
+#define	DTRACE_INSCRATCH(mstate, alloc_sz) \
+	((mstate)->dtms_scratch_base + (mstate)->dtms_scratch_size - \
+	(mstate)->dtms_scratch_ptr >= (alloc_sz))
 
 #define	DTRACE_LOADFUNC(bits)						\
 /*CSTYLED*/								\
@@ -2932,8 +2945,13 @@
 		 * probes will not activate in user contexts to which the
 		 * enabling user does not have permissions.
 		 */
-		if (mstate->dtms_scratch_ptr + scratch_size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+
+		/*
+		 * Rounding up the user allocation size could have overflowed
+		 * a large, bogus allocation (like -1ULL) to 0.
+		 */
+		if (scratch_size < size ||
+		    !DTRACE_INSCRATCH(mstate, scratch_size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -2983,8 +3001,7 @@
 		 * probes will not activate in user contexts to which the
 		 * enabling user does not have permissions.
 		 */
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3330,8 +3347,7 @@
 			break;
 		}
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3440,8 +3456,7 @@
 		if (nargs <= 2)
 			remaining = (int64_t)size;
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3516,8 +3531,7 @@
 			regs[rd] = NULL;
 		}
 
-		if (size == 0 || mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3697,8 +3711,7 @@
 			break;
 		}
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3742,8 +3755,7 @@
 		uint64_t size = 22;	/* enough room for 2^64 in decimal */
 		char *end = (char *)mstate->dtms_scratch_ptr + size - 1;
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3807,8 +3819,7 @@
 			break;
 		}
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -3936,8 +3947,7 @@
 			break;
 		}
 
-		if (mstate->dtms_scratch_ptr + size >
-		    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+		if (!DTRACE_INSCRATCH(mstate, size)) {
 			DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 			regs[rd] = NULL;
 			break;
@@ -4731,17 +4741,21 @@
 			uintptr_t ptr = P2ROUNDUP(mstate->dtms_scratch_ptr, 8);
 			size_t size = ptr - mstate->dtms_scratch_ptr + regs[r1];
 
-			if (mstate->dtms_scratch_ptr + size >
-			    mstate->dtms_scratch_base +
-			    mstate->dtms_scratch_size) {
+			/*
+			 * Rounding up the user allocation size could have
+			 * overflowed large, bogus allocations (like -1ULL) to
+			 * 0.
+			 */
+			if (size < regs[r1] ||
+			    !DTRACE_INSCRATCH(mstate, size)) {
 				DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH);
 				regs[rd] = NULL;
-			} else {
-				dtrace_bzero((void *)
-				    mstate->dtms_scratch_ptr, size);
-				mstate->dtms_scratch_ptr += size;
-				regs[rd] = ptr;
-			}
+				break;
+			}
+
+			dtrace_bzero((void *) mstate->dtms_scratch_ptr, size);
+			mstate->dtms_scratch_ptr += size;
+			regs[rd] = ptr;
 			break;
 		}
 
@@ -5019,8 +5033,7 @@
 	size = (uintptr_t)fps - mstate->dtms_scratch_ptr +
 	    (nframes * sizeof (uint64_t));
 
-	if (mstate->dtms_scratch_ptr + size >
-	    mstate->dtms_scratch_base + mstate->dtms_scratch_size) {
+	if (!DTRACE_INSCRATCH(mstate, size)) {
 		/*
 		 * Not enough room for our frame pointers -- need to indicate
 		 * that we ran out of scratch space.