From 4cc9f98e4c10741a28cd0c715e6c130cb30dbd0f Mon Sep 17 00:00:00 2001 From: guoweis Date: Wed, 24 Feb 2016 11:10:06 -0800 Subject: [PATCH] Fix bug 574524: DtlsTransportChannel crashes after SSL closes remotely When remote side closes, opensslstreamadapter could return SR_EOS which will not trigger upper layer to clean up what's left in the StreamInterfaceChannel. The result of this is when there are more packets coming in, the Write on the StreamInterfaceChannel will overflow the buffer. The fix here is that when receiving the remote side close signal, we also close the underneath StreamInterfaceChannel which will clean up the queue to prevent overflow. BUG=574524 TBR=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1566023002 Cr-Commit-Position: refs/heads/master@{#11751} --- webrtc/base/bufferqueue.cc | 8 ++++++++ webrtc/base/bufferqueue.h | 3 +++ webrtc/base/opensslstreamadapter.cc | 4 ++++ webrtc/p2p/base/dtlstransportchannel.cc | 5 +++++ webrtc/p2p/base/dtlstransportchannel.h | 2 +- 5 files changed, 21 insertions(+), 1 deletion(-) diff --git a/webrtc/base/bufferqueue.cc b/webrtc/base/bufferqueue.cc index 9c2324e768..6e5439da26 100644 --- a/webrtc/base/bufferqueue.cc +++ b/webrtc/base/bufferqueue.cc @@ -34,6 +34,14 @@ size_t BufferQueue::size() const { return queue_.size(); } +void BufferQueue::Clear() { + CritScope cs(&crit_); + while (!queue_.empty()) { + free_list_.push_back(queue_.front()); + queue_.pop_front(); + } +} + bool BufferQueue::ReadFront(void* buffer, size_t bytes, size_t* bytes_read) { CritScope cs(&crit_); if (queue_.empty()) { diff --git a/webrtc/base/bufferqueue.h b/webrtc/base/bufferqueue.h index 89ffdf0189..623d85a384 100644 --- a/webrtc/base/bufferqueue.h +++ b/webrtc/base/bufferqueue.h @@ -28,6 +28,9 @@ class BufferQueue { // Return number of queued buffers. size_t size() const; + // Clear the BufferQueue by moving all Buffers from |queue_| to |free_list_|. + void Clear(); + // ReadFront will only read one buffer at a time and will truncate buffers // that don't fit in the passed memory. // Returns true unless no data could be returned. diff --git a/webrtc/base/opensslstreamadapter.cc b/webrtc/base/opensslstreamadapter.cc index 4a1dc80821..30326f57a2 100644 --- a/webrtc/base/opensslstreamadapter.cc +++ b/webrtc/base/opensslstreamadapter.cc @@ -636,6 +636,10 @@ StreamResult OpenSSLStreamAdapter::Read(void* data, size_t data_len, return SR_BLOCK; case SSL_ERROR_ZERO_RETURN: LOG(LS_VERBOSE) << " -- remote side closed"; + // When we're closed at SSL layer, also close the stream level which + // performs necessary clean up. Otherwise, a new incoming packet after + // this could overflow the stream buffer. + this->stream()->Close(); return SR_EOS; break; default: diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index 59ef2bc41e..2185fb3aad 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -88,6 +88,11 @@ bool StreamInterfaceChannel::OnPacketReceived(const char* data, size_t size) { return ret; } +void StreamInterfaceChannel::Close() { + packets_.Clear(); + state_ = rtc::SS_CLOSED; +} + DtlsTransportChannelWrapper::DtlsTransportChannelWrapper( TransportChannelImpl* channel) : TransportChannelImpl(channel->transport_name(), channel->component()), diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index f396a57d30..ad30441a15 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -34,7 +34,7 @@ class StreamInterfaceChannel : public rtc::StreamInterface { // Implementations of StreamInterface rtc::StreamState GetState() const override { return state_; } - void Close() override { state_ = rtc::SS_CLOSED; } + void Close() override; rtc::StreamResult Read(void* buffer, size_t buffer_len, size_t* read,