From 967e55306e7bb0a58a7cf2c5b905a2608f395875 Mon Sep 17 00:00:00 2001 From: pooryorick Date: Mon, 13 Mar 2023 12:22:06 +0000 Subject: Fix for issue [ea69b0258a9833cb], crash when using a channel transformation on TCP client socket. --- generic/tclIO.c | 38 ++++++++++++++---------- tests/ioTrans.test | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 17 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 85ff39b..715f8c7 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -8551,6 +8551,7 @@ UpdateInterest( mask &= ~TCL_EXCEPTION; if (!statePtr->timer) { + TclChannelPreserve((Tcl_Channel)chanPtr); statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, ChannelTimerProc, chanPtr); } @@ -8584,23 +8585,28 @@ ChannelTimerProc( ChannelState *statePtr = chanPtr->state; /* State info for channel */ - if (!GotFlag(statePtr, CHANNEL_NEED_MORE_DATA) - && (statePtr->interestMask & TCL_READABLE) - && (statePtr->inQueueHead != NULL) - && IsBufferReady(statePtr->inQueueHead)) { - /* - * Restart the timer in case a channel handler reenters the event loop - * before UpdateInterest gets called by Tcl_NotifyChannel. - */ - - statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, - ChannelTimerProc,chanPtr); - Tcl_Preserve(statePtr); - Tcl_NotifyChannel((Tcl_Channel) chanPtr, TCL_READABLE); - Tcl_Release(statePtr); + if (chanPtr->typePtr == NULL) { + TclChannelRelease((Tcl_Channel)chanPtr); } else { - statePtr->timer = NULL; - UpdateInterest(chanPtr); + if (!GotFlag(statePtr, CHANNEL_NEED_MORE_DATA) + && (statePtr->interestMask & TCL_READABLE) + && (statePtr->inQueueHead != NULL) + && IsBufferReady(statePtr->inQueueHead)) { + /* + * Restart the timer in case a channel handler reenters the event loop + * before UpdateInterest gets called by Tcl_NotifyChannel. + */ + + statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, + ChannelTimerProc,chanPtr); + Tcl_Preserve(statePtr); + Tcl_NotifyChannel((Tcl_Channel) chanPtr, TCL_READABLE); + Tcl_Release(statePtr); + } else { + statePtr->timer = NULL; + UpdateInterest(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); + } } } diff --git a/tests/ioTrans.test b/tests/ioTrans.test index f185117..130ff80 100644 --- a/tests/ioTrans.test +++ b/tests/ioTrans.test @@ -634,6 +634,58 @@ test iortrans-4.9 {chan read, gets, bug 2921116} -setup { } } + + +namespace eval reflector { + proc initialize {_ chan mode} { + return {initialize finalize watch read} + } + + + proc finalize {_ chan} { + namespace delete $_ + } + + + proc read {_ chan count} { + namespace upvar $_ source source + set res [string range $source 0 $count-1] + set source [string range $source $count end] + return $res + } + + + proc watch {_ chan events} { + after 0 [list chan postevent $chan read] + return read + } + + namespace ensemble create -parameters _ + namespace export * +} + + + + +namespace eval inputfilter { + proc initialize {chan mode} { + return {initialize finalize read} + } + + proc read {chan buffer} { + return $buffer + } + + proc finalize chan { + namespace delete $chan + } + + namespace ensemble create + namespace export * +} + + + # Channel read transform that is just the identity - pass all through proc idxform {cmd handle args} { switch -- $cmd { @@ -2089,7 +2141,39 @@ test iortrans.tf-11.1 {origin thread of moved transform destroyed during access} thread::release $tidb } -result {Owner lost} -# ### ### ### ######### ######### ######### + +test iortrans-ea69b0258a9833cb { + Crash when using a channel transformation on TCP client socket + + "line two" does not make it into result. This issue should probably be + addressed, but it is outside the scope of this test. +} -setup { + set res {} + set read 0 +} -body { + namespace eval reflector1 { + variable source "line one\nline two" + interp alias {} [namespace current]::dispatch {} [ + namespace parent]::reflector [namespace current] + } + set chan [chan create read [namespace which reflector1::dispatch]] + chan configure $chan -blocking 0 + chan push $chan inputfilter + chan event $chan read [list ::apply [list chan { + variable res + variable read + set gets [gets $chan] + append res $gets + incr read + } [namespace current]] $chan] + vwait [namespace current]::read + chan pop $chan + vwait [namespace current]::read + return $res +} -cleanup { + catch {unset read} + close $chan +} -result {line one} cleanupTests return -- cgit v0.12 From 18a99f2522b77516b62a0d44dca1c90b3479bda1 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Tue, 14 Mar 2023 10:05:24 +0000 Subject: Add "ucs-2" constraint to encoding-bug-183a1adcc0-5 testcase, otherwise it fails with TCL_UTF_MAX>3. Broken by [47857515422b8519|this] commit --- tests/encoding.test | 2 +- tests/ioTrans.test | 2 +- win/tclWinTest.c | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/encoding.test b/tests/encoding.test index 26efb19..bac80c9 100644 --- a/tests/encoding.test +++ b/tests/encoding.test @@ -768,7 +768,7 @@ test encoding-bug-183a1adcc0-4 {Bug [183a1adcc0] Buffer overflow Tcl_UtfToExtern } -result [list 0 [list nospace {} \x00\x00\xff]] test encoding-bug-183a1adcc0-5 {Bug [183a1adcc0] Buffer overflow Tcl_UtfToExternal} -constraints { - testencoding + testencoding ucs-2 } -body { # Note - buffers are initialized to \xff list [catch {testencoding Tcl_UtfToExternal unicode A {start end} {} 4} result] $result diff --git a/tests/ioTrans.test b/tests/ioTrans.test index 130ff80..3a23e61 100644 --- a/tests/ioTrans.test +++ b/tests/ioTrans.test @@ -671,7 +671,7 @@ namespace eval inputfilter { proc initialize {chan mode} { return {initialize finalize read} } - + proc read {chan buffer} { return $buffer } diff --git a/win/tclWinTest.c b/win/tclWinTest.c index d70d217..6ca49f6 100644 --- a/win/tclWinTest.c +++ b/win/tclWinTest.c @@ -419,7 +419,7 @@ TestplatformChmod( const char *nativePath, int pmode) { - /* + /* * Note FILE_DELETE_CHILD missing from dirWriteMask because we do * not want overriding of child's delete setting when testing */ @@ -427,7 +427,7 @@ TestplatformChmod( FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_ADD_FILE | FILE_ADD_SUBDIRECTORY | STANDARD_RIGHTS_WRITE | DELETE | SYNCHRONIZE; - static const DWORD dirReadMask = + static const DWORD dirReadMask = FILE_READ_ATTRIBUTES | FILE_READ_EA | FILE_LIST_DIRECTORY | STANDARD_RIGHTS_READ | SYNCHRONIZE; /* Note - default user privileges allow ignoring TRAVERSE setting */ @@ -437,7 +437,7 @@ TestplatformChmod( static const DWORD fileWriteMask = FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_WRITE_DATA | FILE_APPEND_DATA | STANDARD_RIGHTS_WRITE | DELETE | SYNCHRONIZE; - static const DWORD fileReadMask = + static const DWORD fileReadMask = FILE_READ_ATTRIBUTES | FILE_READ_EA | FILE_READ_DATA | STANDARD_RIGHTS_READ | SYNCHRONIZE; static const DWORD fileExecuteMask = @@ -471,7 +471,7 @@ TestplatformChmod( if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &hToken)) { goto done; } - + /* Get process SID */ if (!GetTokenInformation(hToken, TokenUser, NULL, 0, &dw) && GetLastError() != ERROR_INSUFFICIENT_BUFFER) { @@ -489,7 +489,7 @@ TestplatformChmod( ckfree(aceEntry[nSids].pSid); /* Since we have not ++'ed nSids */ goto done; } - /* + /* * Always include DACL modify rights so we don't get locked out */ aceEntry[nSids].mask = READ_CONTROL | WRITE_DAC | WRITE_OWNER | SYNCHRONIZE | -- cgit v0.12