admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete a cricket::DtlsTransport when PC is closed

Browse Source 
This avoids use-after-free problems that occur when references
to webrtc::DtlsTransport objects are held outside of the PC.

Bug: chromium:907849
Change-Id: Id428c8e616482eff0f4327d2eac17e29bb3f6484
Reviewed-on: https://webrtc-review.googlesource.com/c/113303
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25915}
This commit is contained in:
Harald Alvestrand 2018-12-06 10:55:20 +01:00 committed by Commit Bot
parent fafae11bfc
commit 628f37a6fe
3 changed files with 26 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pc/dtlstransport.h
View File

@ -27,6 +27,7 @@ class DtlsTransport : public DtlsTransportInterface {
cricket::DtlsTransportInternal* internal() {
return internal_dtls_transport_.get();
}
void clear() { internal_dtls_transport_.reset(); }
private:
std::unique_ptr<cricket::DtlsTransportInternal> internal_dtls_transport_;

16
pc/jseptransport.cc
View File

@ -136,6 +136,12 @@ JsepTransport::~JsepTransport() {
if (media_transport_) {
media_transport_->SetMediaTransportStateCallback(nullptr);
}
// Clear all DtlsTransports. There may be pointers to these from
// other places, so we can't assume they'll be deleted by the destructor.
rtp_dtls_transport_->clear();
if (rtcp_dtls_transport_) {
rtcp_dtls_transport_->clear();
}
}
webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
@ -192,9 +198,11 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
}
}
RTC_DCHECK(rtp_dtls_transport_->internal());
SetLocalIceParameters(rtp_dtls_transport_->internal()->ice_transport());
if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal());
SetLocalIceParameters(rtcp_dtls_transport_->internal()->ice_transport());
}
@ -255,9 +263,11 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription(
}
remote_description_.reset(new JsepTransportDescription(jsep_description));
RTC_DCHECK(rtp_dtls_transport_->internal());
SetRemoteIceParameters(rtp_dtls_transport_->internal()->ice_transport());
if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal());
SetRemoteIceParameters(rtcp_dtls_transport_->internal()->ice_transport());
}
@ -292,6 +302,7 @@ webrtc::RTCError JsepTransport::AddRemoteCandidates(
"Candidate has an unknown component: " +
candidate.ToString() + " for mid " + mid());
}
RTC_DCHECK(transport->internal() && transport->internal()->ice_transport());
transport->internal()->ice_transport()->AddRemoteCandidate(candidate);
}
return webrtc::RTCError::OK();
@ -306,6 +317,7 @@ void JsepTransport::SetNeedsIceRestartFlag() {
absl::optional<rtc::SSLRole> JsepTransport::GetDtlsRole() const {
RTC_DCHECK(rtp_dtls_transport_);
RTC_DCHECK(rtp_dtls_transport_->internal());
rtc::SSLRole dtls_role;
if (!rtp_dtls_transport_->internal()->GetDtlsRole(&dtls_role)) {
return absl::optional<rtc::SSLRole>();
@ -317,8 +329,10 @@ absl::optional<rtc::SSLRole> JsepTransport::GetDtlsRole() const {
bool JsepTransport::GetStats(TransportStats* stats) {
stats->transport_name = mid();
stats->channel_stats.clear();
RTC_DCHECK(rtp_dtls_transport_->internal());
bool ret = GetTransportStats(rtp_dtls_transport_->internal(), stats);
if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal());
ret &= GetTransportStats(rtcp_dtls_transport_->internal(), stats);
}
return ret;
@ -534,6 +548,7 @@ webrtc::RTCError JsepTransport::NegotiateAndSetDtlsParameters(
// between future SetRemote/SetLocal invocations and new transport
// creation, we have the negotiation state saved until a new
// negotiation happens.
RTC_DCHECK(rtp_dtls_transport_->internal());
webrtc::RTCError error = SetNegotiatedDtlsParameters(
rtp_dtls_transport_->internal(), negotiated_dtls_role,
remote_fingerprint.get());
@ -542,6 +557,7 @@ webrtc::RTCError JsepTransport::NegotiateAndSetDtlsParameters(
}
if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal());
error = SetNegotiatedDtlsParameters(rtcp_dtls_transport_->internal(),
negotiated_dtls_role,
remote_fingerprint.get());

9
pc/jseptransportcontroller_unittest.cc
View File

@ -385,6 +385,15 @@ TEST_F(JsepTransportControllerTest, GetDtlsTransport) {
EXPECT_EQ(nullptr, transport_controller_->GetRtcpDtlsTransport(kVideoMid2));
EXPECT_EQ(nullptr,
transport_controller_->LookupDtlsTransportByMid(kVideoMid2));
// Take a pointer to a transport, shut down the transport controller,
// and verify that the resulting container is empty.
auto dtls_transport =
transport_controller_->LookupDtlsTransportByMid(kVideoMid1);
webrtc::DtlsTransport* my_transport =
static_cast<DtlsTransport*>(dtls_transport.get());
EXPECT_NE(nullptr, my_transport->internal());
transport_controller_.reset();
EXPECT_EQ(nullptr, my_transport->internal());
}
TEST_F(JsepTransportControllerTest, GetDtlsTransportWithRtcpMux) {