From 391cc5529b10b842b5e34acb19bf559b56df7f49 Mon Sep 17 00:00:00 2001 From: dkf Date: Sat, 17 Oct 2009 22:35:58 +0000 Subject: Fix [Bug 2629338]: Stop evil unset traces from accessing freed memory. --- ChangeLog | 16 ++++++++++++---- generic/tclTrace.c | 3 ++- generic/tclVar.c | 30 +++++++++++++++++++++++++----- 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 + + * 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 * generic/tclDictObj.c (DictIncrCmd): [Bug 2874678]: Don't leak any @@ -12,10 +20,10 @@ functions more consistent. Patch supplied by Joe Mistachkin . - * 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 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; } /* -- cgit v0.12