Fix [Bug 2629338]: Stop evil unset traces from accessing freed memory.

3 files changed, 39 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 99b598d..78300b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-10-17  Donal K. Fellows  <dkf@users.sf.net>
+
+	* generic/tclVar.c (TclDeleteCompiledLocalVars, UnsetVarStruct)
+	(TclDeleteNamespaceVars): 
+	* generic/tclTrace.c (Tcl_UntraceVar2): [Bug 2629338]: Stop traces
+	that are deleted part way through (a feature used by tdom) from
+	causing freed memory to be accessed.
+
 2009-10-08  Donal K. Fellows  <dkf@users.sf.net>
 
 	* generic/tclDictObj.c (DictIncrCmd): [Bug 2874678]: Don't leak any
@@ -12,10 +20,10 @@
 	functions more consistent. Patch supplied by Joe Mistachkin
 	<joe@mistachkin.com>.
 
-	* generic/tclIORChan.c (ErrnoReturn): Replace the hardwired
-	  constant 11 with the proper errno define, EAGAIN. What was I
-	  thinking ? The BSD's have a different errno assignment and break
-	  with the hardwired number. Reported by emiliano on the chat.
+	* generic/tclIORChan.c (ErrnoReturn): Replace the hardwired constant
+	11 with the proper errno define, EAGAIN. What was I thinking? The
+	BSD's have a different errno assignment and break with the hardwired
+	number. Reported by emiliano on the chat.
 
 2009-10-06  Don Porter  <dgp@users.sourceforge.net>
 
diff --git a/generic/tclTrace.c b/generic/tclTrace.c
index bc6d289..346defc 100644
--- a/generic/tclTrace.c
+++ b/generic/tclTrace.c
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclTrace.c,v 1.47.2.1 2008/10/08 14:52:39 dgp Exp $
+ * RCS: @(#) $Id: tclTrace.c,v 1.47.2.2 2009/10/17 22:35:58 dkf Exp $
  */
 
 #include "tclInt.h"
@@ -2894,6 +2894,7 @@ Tcl_UntraceVar2(
     } else {
 	prevPtr->nextPtr = nextPtr;
     }
+    tracePtr->nextPtr = NULL;
     Tcl_EventuallyFree((ClientData) tracePtr, TCL_DYNAMIC);
 
     for (tracePtr = nextPtr; tracePtr != NULL;
diff --git a/generic/tclVar.c b/generic/tclVar.c
index 3a3a10f..969cc17 100644
--- a/generic/tclVar.c
+++ b/generic/tclVar.c
@@ -16,7 +16,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclVar.c,v 1.160.2.5 2009/08/25 21:01:05 andreas_kupries Exp $
+ * RCS: @(#) $Id: tclVar.c,v 1.160.2.6 2009/10/17 22:35:58 dkf Exp $
  */
 
 #include "tclInt.h"
@@ -2364,11 +2364,22 @@ UnsetVarStruct(
 	if ((dummyVar.flags & VAR_TRACED_UNSET)
 		|| (arrayPtr && (arrayPtr->flags & VAR_TRACED_UNSET))) {
 	    dummyVar.flags &= ~VAR_TRACE_ACTIVE;
-	    TclObjCallVarTraces(iPtr, arrayPtr, (Var *) &dummyVar,
-		    part1Ptr, part2Ptr,
+	    TclObjCallVarTraces(iPtr, arrayPtr, &dummyVar, part1Ptr, part2Ptr,
 		    (flags & (TCL_GLOBAL_ONLY|TCL_NAMESPACE_ONLY))
 			    | TCL_TRACE_UNSETS,
 		    /* leaveErrMsg */ 0, -1);
+
+	    /*
+	     * The traces that we just called may have triggered a change in
+	     * the set of traces. [Bug 2629338]
+	     */
+
+	    tracePtr = NULL;
+	    if (TclIsVarTraced(&dummyVar)) {
+		tPtr = Tcl_FindHashEntry(&iPtr->varTraces, (char *) &dummyVar);
+		tracePtr = Tcl_GetHashValue(tPtr);
+	    }
+
 	    if (tPtr) {
 		Tcl_DeleteHashEntry(tPtr);
 	    }
@@ -2381,6 +2392,7 @@ UnsetVarStruct(
 		VarTrace *prevPtr = tracePtr;
 
 		tracePtr = tracePtr->nextPtr;
+		prevPtr->nextPtr = NULL;
 		Tcl_EventuallyFree((ClientData) prevPtr, TCL_DYNAMIC);
 	    }
 	    for (activePtr = iPtr->activeVarTracePtr;  activePtr != NULL;
@@ -4382,7 +4394,7 @@ TclDeleteNamespaceVars(
 	    varPtr = VarHashFirstVar(tablePtr, &search)) {
 	Tcl_Obj *objPtr = Tcl_NewObj();
 	Tcl_IncrRefCount(objPtr);
-	
+
 	VarHashRefCount(varPtr)++;	/* Make sure we get to remove from
 					 * hash. */
 	Tcl_GetVariableFullName(interp, (Tcl_Var) varPtr, objPtr);
@@ -4390,13 +4402,13 @@ TclDeleteNamespaceVars(
 		NULL, flags);
 	Tcl_DecrRefCount(objPtr); /* free no longer needed obj */
 
-
 	/*
 	 * Remove the variable from the table and force it undefined in case
 	 * an unset trace brought it back from the dead.
 	 */
 
 	if (TclIsVarTraced(varPtr)) {
+	    ActiveVarTrace *activePtr;
 	    Tcl_HashEntry *tPtr = Tcl_FindHashEntry(&iPtr->varTraces,
 		    (char *) varPtr);
 	    VarTrace *tracePtr = (VarTrace *) Tcl_GetHashValue(tPtr);
@@ -4405,10 +4417,17 @@ TclDeleteNamespaceVars(
 		VarTrace *prevPtr = tracePtr;
 
 		tracePtr = tracePtr->nextPtr;
+		prevPtr->nextPtr = NULL;
 		Tcl_EventuallyFree((ClientData) prevPtr, TCL_DYNAMIC);
 	    }
 	    Tcl_DeleteHashEntry(tPtr);
 	    varPtr->flags &= ~VAR_ALL_TRACES;
+	    for (activePtr = iPtr->activeVarTracePtr; activePtr != NULL;
+		    activePtr = activePtr->nextPtr) {
+		if (activePtr->varPtr == varPtr) {
+		    activePtr->nextTracePtr = NULL;
+		}
+	    }
 	}
 	VarHashRefCount(varPtr)--;
 	VarHashDeleteEntry(varPtr);
@@ -4511,6 +4530,7 @@ TclDeleteCompiledLocalVars(
 	UnsetVarStruct(varPtr, NULL, iPtr, *namePtrPtr, NULL,
 		TCL_TRACE_UNSETS);
     }
+    framePtr->numCompiledLocals = 0;
 }
 
 /*