When scheduling a delayed task, one receives a TaskToken. This token can be used lateron for cancelling the task. Or the task could be run in the interim and the token could be reissued for a different task. If the task is unscheduled at this point, this consitutues a use-after-free scenario. According to http://lists.live555.com/pipermail/live-devel/2016-August/020225.html one is not supposed to unschedule such a task.
Accordingly, we update all task handler functions to clear their corresponding TaskToken once they are run. A subsequent call to unscheduleDelayedTask will harmlessly unschedule the NULL token. Note that clearing a token after unscheduleDelayedTask is not necessary. --- .../WindowsAudioInputDevice_common.cpp | 3 ++- liveMedia/GenericMediaServer.cpp | 1 + liveMedia/ProxyServerMediaSession.cpp | 22 +++++++++++++------ liveMedia/SIPClient.cpp | 3 +++ liveMedia/T140TextRTPSink.cpp | 2 +- liveMedia/include/ProxyServerMediaSession.hh | 1 + testProgs/playCommon.cpp | 4 ++++ 7 files changed, 27 insertions(+), 9 deletions(-) This is reviving a pretty old thread. Yet, the problem reported there persists and deserves a fix. This followup is meant to close the matter for good. Helmut diff --git a/WindowsAudioInputDevice/WindowsAudioInputDevice_common.cpp b/WindowsAudioInputDevice/WindowsAudioInputDevice_common.cpp index 451ef4e7..6534c43e 100644 --- a/WindowsAudioInputDevice/WindowsAudioInputDevice_common.cpp +++ b/WindowsAudioInputDevice/WindowsAudioInputDevice_common.cpp @@ -65,7 +65,7 @@ void WindowsAudioInputDevice_common::doGetNextFrame() { void WindowsAudioInputDevice_common::doStopGettingFrames() { // Turn off the audio poller: - envir().taskScheduler().unscheduleDelayedTask(nextTask()); nextTask() = NULL; + envir().taskScheduler().unscheduleDelayedTask(nextTask()); } double WindowsAudioInputDevice_common::getAverageLevel() const { @@ -96,6 +96,7 @@ double WindowsAudioInputDevice_common::getAverageLevel() const { void WindowsAudioInputDevice_common::audioReadyPoller(void* clientData) { WindowsAudioInputDevice_common* inputDevice = (WindowsAudioInputDevice_common*)clientData; + inputDevice->nextTask() = NULL; inputDevice->audioReadyPoller1(); } diff --git a/liveMedia/GenericMediaServer.cpp b/liveMedia/GenericMediaServer.cpp index 7e970f28..33eab479 100644 --- a/liveMedia/GenericMediaServer.cpp +++ b/liveMedia/GenericMediaServer.cpp @@ -303,6 +303,7 @@ void GenericMediaServer::ClientSession::noteClientLiveness(ClientSession* client } void GenericMediaServer::ClientSession::livenessTimeoutTask(ClientSession* clientSession) { + clientSession->fLivenessCheckTask = NULL; // If this gets called, the client session is assumed to have timed out, so delete it: #ifdef DEBUG char const* streamName diff --git a/liveMedia/ProxyServerMediaSession.cpp b/liveMedia/ProxyServerMediaSession.cpp index 512e4b47..eb6cda15 100644 --- a/liveMedia/ProxyServerMediaSession.cpp +++ b/liveMedia/ProxyServerMediaSession.cpp @@ -115,7 +115,7 @@ ProxyServerMediaSession tunnelOverHTTPPortNum, verbosityLevel > 0 ? verbosityLevel-1 : verbosityLevel, socketNumToServer); - ProxyRTSPClient::sendDESCRIBE(fProxyRTSPClient); + fProxyRTSPClient->sendDESCRIBE(); } ProxyServerMediaSession::~ProxyServerMediaSession() { @@ -258,10 +258,10 @@ ProxyRTSPClient::ProxyRTSPClient(ProxyServerMediaSession& ourServerMediaSession, } void ProxyRTSPClient::reset() { - envir().taskScheduler().unscheduleDelayedTask(fLivenessCommandTask); fLivenessCommandTask = NULL; - envir().taskScheduler().unscheduleDelayedTask(fDESCRIBECommandTask); fDESCRIBECommandTask = NULL; - envir().taskScheduler().unscheduleDelayedTask(fSubsessionTimerTask); fSubsessionTimerTask = NULL; - envir().taskScheduler().unscheduleDelayedTask(fResetTask); fResetTask = NULL; + envir().taskScheduler().unscheduleDelayedTask(fLivenessCommandTask); + envir().taskScheduler().unscheduleDelayedTask(fDESCRIBECommandTask); + envir().taskScheduler().unscheduleDelayedTask(fSubsessionTimerTask); + envir().taskScheduler().unscheduleDelayedTask(fResetTask); fSetupQueueHead = fSetupQueueTail = NULL; fNumSetupsDone = 0; @@ -419,6 +419,7 @@ void ProxyRTSPClient::scheduleLivenessCommand() { void ProxyRTSPClient::sendLivenessCommand(void* clientData) { ProxyRTSPClient* rtspClient = (ProxyRTSPClient*)clientData; + rtspClient->fLivenessCommandTask = NULL; // Note. By default, we do not send "GET_PARAMETER" as our 'liveness notification' command, even if the server previously // indicated (in its response to our earlier "OPTIONS" command) that it supported "GET_PARAMETER". This is because @@ -444,6 +445,7 @@ void ProxyRTSPClient::scheduleReset() { } void ProxyRTSPClient::doReset() { + fResetTask = NULL; if (fVerbosityLevel > 0) { envir() << *this << "::doReset\n"; } @@ -452,7 +454,7 @@ void ProxyRTSPClient::doReset() { fOurServerMediaSession.resetDESCRIBEState(); setBaseURL(fOurURL); // because we'll be sending an initial "DESCRIBE" all over again - sendDESCRIBE(this); + sendDESCRIBE(); } void ProxyRTSPClient::doReset(void* clientData) { @@ -478,7 +480,12 @@ void ProxyRTSPClient::scheduleDESCRIBECommand() { void ProxyRTSPClient::sendDESCRIBE(void* clientData) { ProxyRTSPClient* rtspClient = (ProxyRTSPClient*)clientData; - if (rtspClient != NULL) rtspClient->sendDescribeCommand(::continueAfterDESCRIBE, rtspClient->auth()); + rtspClient->fDESCRIBECommandTask = NULL; + rtspClient->sendDESCRIBE(); +} + +void ProxyRTSPClient::sendDESCRIBE() { + sendDescribeCommand(::continueAfterDESCRIBE, auth()); } void ProxyRTSPClient::subsessionTimeout(void* clientData) { @@ -486,6 +493,7 @@ void ProxyRTSPClient::subsessionTimeout(void* clientData) { } void ProxyRTSPClient::handleSubsessionTimeout() { + fSubsessionTimerTask = NULL; // We still have one or more subsessions ('tracks') left to "SETUP". But we can't wait any longer for them. Send a "PLAY" now: MediaSession* sess = fOurServerMediaSession.fClientMediaSession; if (sess != NULL) sendPlayCommand(*sess, ::continueAfterPLAY, -1.0f, -1.0f, 1.0f, fOurAuthenticator); diff --git a/liveMedia/SIPClient.cpp b/liveMedia/SIPClient.cpp index 72e922f3..cab24161 100644 --- a/liveMedia/SIPClient.cpp +++ b/liveMedia/SIPClient.cpp @@ -339,6 +339,7 @@ unsigned const timerDFires = 0xDDDDDDDD; void SIPClient::timerAHandler(void* clientData) { SIPClient* client = (SIPClient*)clientData; + client->fTimerA = NULL; if (client->fVerbosityLevel >= 1) { client->envir() << "RETRANSMISSION " << ++client->fTimerACount << ", after " << client->fTimerALen/1000000.0 @@ -349,6 +350,7 @@ void SIPClient::timerAHandler(void* clientData) { void SIPClient::timerBHandler(void* clientData) { SIPClient* client = (SIPClient*)clientData; + client->fTimerB = NULL; if (client->fVerbosityLevel >= 1) { client->envir() << "RETRANSMISSION TIMEOUT, after " << 64*client->fT1/1000000.0 << " seconds\n"; @@ -359,6 +361,7 @@ void SIPClient::timerBHandler(void* clientData) { void SIPClient::timerDHandler(void* clientData) { SIPClient* client = (SIPClient*)clientData; + client->fTimerD = NULL; if (client->fVerbosityLevel >= 1) { client->envir() << "TIMER D EXPIRED\n"; } diff --git a/liveMedia/T140TextRTPSink.cpp b/liveMedia/T140TextRTPSink.cpp index 6d9212bd..84554fba 100644 --- a/liveMedia/T140TextRTPSink.cpp +++ b/liveMedia/T140TextRTPSink.cpp @@ -143,6 +143,7 @@ void T140IdleFilter::handleIdleTimeout(void* clientData) { } void T140IdleFilter::handleIdleTimeout() { + fIdleTimerTask = NULL; // No data has arrived from the upstream source within our specified 'idle period' (after data was requested from downstream). // Send an empty 'idle' frame to our downstream "T140TextRTPSink". (This will cause an empty RTP packet to get sent.) deliverEmptyFrame(); @@ -178,7 +179,6 @@ void T140IdleFilter::onSourceClosure(void* clientData) { void T140IdleFilter::onSourceClosure() { envir().taskScheduler().unscheduleDelayedTask(fIdleTimerTask); - fIdleTimerTask = NULL; handleClosure(); } diff --git a/liveMedia/include/ProxyServerMediaSession.hh b/liveMedia/include/ProxyServerMediaSession.hh index 902dfda1..ea7b4f70 100644 --- a/liveMedia/include/ProxyServerMediaSession.hh +++ b/liveMedia/include/ProxyServerMediaSession.hh @@ -65,6 +65,7 @@ private: void scheduleDESCRIBECommand(); static void sendDESCRIBE(void* clientData); + void sendDESCRIBE(); static void subsessionTimeout(void* clientData); void handleSubsessionTimeout(); diff --git a/testProgs/playCommon.cpp b/testProgs/playCommon.cpp index 707a4a24..47dbc3ac 100644 --- a/testProgs/playCommon.cpp +++ b/testProgs/playCommon.cpp @@ -1163,6 +1163,7 @@ void sessionTimerHandler(void* /*clientData*/) { } void periodicFileOutputTimerHandler(void* /*clientData*/) { + periodicFileOutputTask = NULL; fileOutputSecondsSoFar += fileOutputInterval; // First, close the existing output files: @@ -1435,6 +1436,7 @@ void signalHandlerShutdown(int /*sig*/) { } void checkForPacketArrival(void* /*clientData*/) { + arrivalCheckTimerTask = NULL; if (!notifyOnPacketArrival) return; // we're not checking // Check each subsession, to see whether it has received data packets: @@ -1493,6 +1495,7 @@ void checkForPacketArrival(void* /*clientData*/) { } void checkInterPacketGaps(void* /*clientData*/) { + interPacketGapCheckTimerTask = NULL; if (interPacketGapMaxTime == 0) return; // we're not checking // Check each subsession, counting up how many packets have been received: @@ -1522,6 +1525,7 @@ void checkInterPacketGaps(void* /*clientData*/) { } void checkSessionTimeoutBrokenServer(void* /*clientData*/) { + sessionTimeoutBrokenServerTask = NULL; if (!sendKeepAlivesToBrokenServers) return; // we're not checking // Send an "OPTIONS" request, starting with the second call -- 2.20.1 _______________________________________________ live-devel mailing list live-devel@lists.live555.com http://lists.live555.com/mailman/listinfo/live-devel