ultimate fix for [79474c58800cdf94]: avoid segfault on copy-state structure freed to early, protected by refCount and by preserving its r/w channels now

1 files changed, 54 insertions, 13 deletions

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 6b77749..8cfa2c8 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -97,6 +97,7 @@ typedef struct GetsState {
 typedef struct CopyState {
     struct Channel *readPtr;	/* Pointer to input channel. */
     struct Channel *writePtr;	/* Pointer to output channel. */
+    int refCount;		/* Reference counter. */
     int readFlags;		/* Original read channel flags. */
     int writeFlags;		/* Original write channel flags. */
     Tcl_WideInt toRead;		/* Number of bytes to copy, or -1. */
@@ -219,6 +220,7 @@ static int		StackSetBlockMode(Channel *chanPtr, int mode);
 static int		SetBlockMode(Tcl_Interp *interp, Channel *chanPtr,
 			    int mode);
 static void		StopCopy(CopyState *csPtr);
+static void		CopyDecrRefCount(CopyState *csPtr);
 static void		TranslateInputEOL(ChannelState *statePtr, char *dst,
 			    const char *src, int *dstLenPtr, int *srcLenPtr);
 static void		UpdateInterest(Channel *chanPtr);
@@ -2081,7 +2083,7 @@ Tcl_UnstackChannel(
 		return TCL_ERROR;
 	    }
 
-	    statePtr->csPtrR  = csPtrR;
+	    statePtr->csPtrR = csPtrR;
 	    statePtr->csPtrW = csPtrW;
 	}
 
@@ -3941,8 +3943,14 @@ Tcl_ClearChannelHandlers(
      * Cancel any pending copy operation.
      */
 
-    StopCopy(statePtr->csPtrR);
-    StopCopy(statePtr->csPtrW);
+    if (statePtr->csPtrR) {
+	StopCopy(statePtr->csPtrR);
+	statePtr->csPtrR = NULL;
+    }
+    if (statePtr->csPtrW) {
+	StopCopy(statePtr->csPtrW);
+	statePtr->csPtrW = NULL;
+    }
 
     /*
      * Must set the interest mask now to 0, otherwise infinite loops will
@@ -9178,6 +9186,9 @@ TclCopyChannel(
     int nonBlocking = (cmdPtr) ? CHANNEL_NONBLOCKING : 0;
     int moveBytes;
 
+    TclChannelPreserve(inChan);
+    TclChannelPreserve(outChan);
+
     inStatePtr = inPtr->state;
     outStatePtr = outPtr->state;
 
@@ -9248,6 +9259,7 @@ TclCopyChannel(
     csPtr->bufSize = !moveBytes * inStatePtr->bufSize;
     csPtr->readPtr = inPtr;
     csPtr->writePtr = outPtr;
+    csPtr->refCount = 2; /* two references below (inStatePtr, outStatePtr) */
     csPtr->readFlags = readFlags;
     csPtr->writeFlags = writeFlags;
     csPtr->toRead = toRead;
@@ -9258,7 +9270,7 @@ TclCopyChannel(
     }
     csPtr->cmdPtr = cmdPtr;
 
-    inStatePtr->csPtrR  = csPtr;
+    inStatePtr->csPtrR = csPtr;
     outStatePtr->csPtrW = csPtr;
 
     if (moveBytes) {
@@ -9272,7 +9284,7 @@ TclCopyChannel(
 
     if ((nonBlocking == CHANNEL_NONBLOCKING) && (toRead == 0)) {
 	Tcl_CreateTimerHandler(0, ZeroTransferTimerProc, csPtr);
-	return 0;
+	return TCL_OK;
     }
 
     /*
@@ -9547,6 +9559,8 @@ CopyData(
 				/* Encoding control */
     int underflow;		/* Input underflow */
 
+    csPtr->refCount++; /* avoid freeing during handling */
+
     inChan	= (Tcl_Channel) csPtr->readPtr;
     outChan	= (Tcl_Channel) csPtr->writePtr;
     inStatePtr	= csPtr->readPtr->state;
@@ -9667,7 +9681,8 @@ CopyData(
 		    TclDecrRefCount(bufObj);
 		    bufObj = NULL;
 		}
-		return TCL_OK;
+		result = TCL_OK;
+		goto done;
 	    }
 	}
 
@@ -9758,7 +9773,8 @@ CopyData(
 		TclDecrRefCount(bufObj);
 		bufObj = NULL;
 	    }
-	    return TCL_OK;
+	    result = TCL_OK;
+	    goto done;
 	}
 
 	/*
@@ -9780,7 +9796,8 @@ CopyData(
 		TclDecrRefCount(bufObj);
 		bufObj = NULL;
 	    }
-	    return TCL_OK;
+	    result = TCL_OK;
+	    goto done;
 	}
     } /* while */
 
@@ -9832,6 +9849,9 @@ CopyData(
 	    }
 	}
     }
+
+done:
+    CopyDecrRefCount(csPtr);
     return result;
 }
 
@@ -9938,8 +9958,6 @@ DoRead(
 	    code = GetInput(chanPtr);
 	    bufPtr = statePtr->inQueueHead;
 
-	    assert(bufPtr != NULL);
-
 	    if (GotFlag(statePtr, CHANNEL_EOF|CHANNEL_BLOCKED)) {
 		/*
 		 * Further reads cannot do any more.
@@ -9948,7 +9966,7 @@ DoRead(
 		break;
 	    }
 
-	    if (code) {
+	    if (code || !bufPtr) {
 		/*
 	     * Read error
 	     */
@@ -10170,9 +10188,32 @@ StopCopy(
 	Tcl_DeleteChannelHandler(inChan, MBEvent, csPtr);
 	Tcl_DeleteChannelHandler(outChan, MBEvent, csPtr);
 	TclDecrRefCount(csPtr->cmdPtr);
+	csPtr->cmdPtr = NULL;
+    }
+
+    if (inStatePtr->csPtrR) {
+	assert(inStatePtr->csPtrR == csPtr);
+	inStatePtr->csPtrR = NULL;
+	CopyDecrRefCount(csPtr);
+    }
+    if (outStatePtr->csPtrW) {
+	assert(outStatePtr->csPtrW == csPtr);
+	outStatePtr->csPtrW = NULL;
+	CopyDecrRefCount(csPtr);
     }
-    inStatePtr->csPtrR = NULL;
-    outStatePtr->csPtrW = NULL;
+}
+
+static void
+CopyDecrRefCount(
+    CopyState *csPtr
+) {
+    if (csPtr->refCount-- > 1) {
+	return;
+    }
+
+    TclChannelRelease((Tcl_Channel)csPtr->readPtr);
+    TclChannelRelease((Tcl_Channel)csPtr->writePtr);
+
     ckfree(csPtr);
 }