From fb648da2b9d33e8641c88a7f0559508539104b6d Mon Sep 17 00:00:00 2001 From: "wu@webrtc.org" Date: Fri, 18 Oct 2013 21:10:51 +0000 Subject: [PATCH] Protect _transportPtr, which can be accessed by different threads in the case of external transport. This change avoid the potential use-after-free, e.g. the case in the reported bug. BUG=2508 RISK=P1 TEST=try bots R=henrika@webrtc.org, xians@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2425004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5000 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/voice_engine/channel.cc | 116 ++++++++++----------------------- 1 file changed, 33 insertions(+), 83 deletions(-) diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 4ef3ed82c..af89d5cd1 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -121,6 +121,8 @@ Channel::SendPacket(int channel, const void *data, int len) WEBRTC_TRACE(kTraceStream, kTraceVoice, VoEId(_instanceId,_channelId), "Channel::SendPacket(channel=%d, len=%d)", channel, len); + CriticalSectionScoped cs(&_callbackCritSect); + if (_transportPtr == NULL) { WEBRTC_TRACE(kTraceError, kTraceVoice, VoEId(_instanceId,_channelId), @@ -158,8 +160,6 @@ Channel::SendPacket(int channel, const void *data, int len) // SRTP or External encryption if (_encrypting) { - CriticalSectionScoped cs(&_callbackCritSect); - if (_encryptionPtr) { if (!_encryptionRTPBufferPtr) @@ -192,39 +192,18 @@ Channel::SendPacket(int channel, const void *data, int len) } } - // Packet transmission using WebRtc socket transport - if (!_externalTransport) - { - int n = _transportPtr->SendPacket(channel, bufferToSendPtr, - bufferLength); - if (n < 0) - { - WEBRTC_TRACE(kTraceError, kTraceVoice, - VoEId(_instanceId,_channelId), - "Channel::SendPacket() RTP transmission using WebRtc" - " sockets failed"); - return -1; - } - return n; - } - - // Packet transmission using external transport transport - { - CriticalSectionScoped cs(&_callbackCritSect); - - int n = _transportPtr->SendPacket(channel, - bufferToSendPtr, - bufferLength); - if (n < 0) - { - WEBRTC_TRACE(kTraceError, kTraceVoice, - VoEId(_instanceId,_channelId), - "Channel::SendPacket() RTP transmission using external" - " transport failed"); - return -1; - } - return n; + int n = _transportPtr->SendPacket(channel, bufferToSendPtr, + bufferLength); + if (n < 0) { + std::string transport_name = + _externalTransport ? "external transport" : "WebRtc sockets"; + WEBRTC_TRACE(kTraceError, kTraceVoice, + VoEId(_instanceId,_channelId), + "Channel::SendPacket() RTP transmission using %s failed", + transport_name.c_str()); + return -1; } + return n; } int @@ -236,16 +215,14 @@ Channel::SendRTCPPacket(int channel, const void *data, int len) WEBRTC_TRACE(kTraceStream, kTraceVoice, VoEId(_instanceId,_channelId), "Channel::SendRTCPPacket(channel=%d, len=%d)", channel, len); + CriticalSectionScoped cs(&_callbackCritSect); + if (_transportPtr == NULL) { - CriticalSectionScoped cs(&_callbackCritSect); - if (_transportPtr == NULL) - { - WEBRTC_TRACE(kTraceError, kTraceVoice, - VoEId(_instanceId,_channelId), - "Channel::SendRTCPPacket() failed to send RTCP packet" - " due to invalid transport object"); - return -1; - } + WEBRTC_TRACE(kTraceError, kTraceVoice, + VoEId(_instanceId,_channelId), + "Channel::SendRTCPPacket() failed to send RTCP packet" + " due to invalid transport object"); + return -1; } uint8_t* bufferToSendPtr = (uint8_t*)data; @@ -262,8 +239,6 @@ Channel::SendRTCPPacket(int channel, const void *data, int len) // SRTP or External encryption if (_encrypting) { - CriticalSectionScoped cs(&_callbackCritSect); - if (_encryptionPtr) { if (!_encryptionRTCPBufferPtr) @@ -294,45 +269,19 @@ Channel::SendRTCPPacket(int channel, const void *data, int len) } } - // Packet transmission using WebRtc socket transport - if (!_externalTransport) - { - int n = _transportPtr->SendRTCPPacket(channel, - bufferToSendPtr, - bufferLength); - if (n < 0) - { - WEBRTC_TRACE(kTraceInfo, kTraceVoice, - VoEId(_instanceId,_channelId), - "Channel::SendRTCPPacket() transmission using WebRtc" - " sockets failed"); - return -1; - } - return n; + int n = _transportPtr->SendRTCPPacket(channel, + bufferToSendPtr, + bufferLength); + if (n < 0) { + std::string transport_name = + _externalTransport ? "external transport" : "WebRtc sockets"; + WEBRTC_TRACE(kTraceInfo, kTraceVoice, + VoEId(_instanceId,_channelId), + "Channel::SendRTCPPacket() transmission using %s failed", + transport_name.c_str()); + return -1; } - - // Packet transmission using external transport transport - { - CriticalSectionScoped cs(&_callbackCritSect); - if (_transportPtr == NULL) - { - return -1; - } - int n = _transportPtr->SendRTCPPacket(channel, - bufferToSendPtr, - bufferLength); - if (n < 0) - { - WEBRTC_TRACE(kTraceInfo, kTraceVoice, - VoEId(_instanceId,_channelId), - "Channel::SendRTCPPacket() transmission using external" - " transport failed"); - return -1; - } - return n; - } - - return len; + return n; } void @@ -5071,6 +5020,7 @@ Channel::GetDeadOrAliveCounters(int& countDead, int& countAlive) const int32_t Channel::SendPacketRaw(const void *data, int len, bool RTCP) { + CriticalSectionScoped cs(&_callbackCritSect); if (_transportPtr == NULL) { return -1;