From ad5465b443cc5fda563dca4b40e52580e14cb621 Mon Sep 17 00:00:00 2001 From: Jeroen de Borst Date: Mon, 1 Oct 2018 15:02:12 -0700 Subject: [PATCH] Deliver partial SCTP messages when the SID is (unexpectedly) changed. We're receiving SCTP messages one chunk at a time. The sender is supposed to set the MSG_EOR bit on the last chunk of a message. A crash (RTC_CHECK) happened when the sender started a new message without indicating the end of the previous message. This is likely due to an SCTP implementation that doesn't (correctly) support the MSG_EOR bit. This change, rather than calling RTC_CHECK, delivers partial messages when SID is (unexpectedly) changed, which is what happened before we paid attention to the MSG_EOR bit ourselves. TBR=pthatcher@webrtc.org Bug: chromium:884926 Change-Id: I18c773bb60ae724b735a83933eedd030f6360a3c Reviewed-on: https://webrtc-review.googlesource.com/c/103023 Commit-Queue: Jeroen de Borst Reviewed-by: Steve Anton Reviewed-by: Qingsi Wang Cr-Commit-Position: refs/heads/master@{#24935} --- media/sctp/sctptransport.cc | 32 ++++++++++++++++++++++---------- media/sctp/sctptransport.h | 3 ++- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/media/sctp/sctptransport.cc b/media/sctp/sctptransport.cc index eaaf63fb3a..a8b945a6f1 100644 --- a/media/sctp/sctptransport.cc +++ b/media/sctp/sctptransport.cc @@ -295,14 +295,31 @@ class SctpTransport::UsrSctpWrapper { } else { ReceiveDataParams params; - // Expect only continuation messages belonging to the same sid, usrsctp - // ensures this. - RTC_CHECK(transport->partial_message_.size() == 0 || - rcv.rcv_sid == transport->partial_message_sid_); + params.sid = rcv.rcv_sid; + params.seq_num = rcv.rcv_ssn; + params.timestamp = rcv.rcv_tsn; + params.type = type; + + // Expect only continuation messages belonging to the same sid, the sctp + // stack should ensure this. + if ((transport->partial_message_.size() != 0) && + (rcv.rcv_sid != transport->partial_params_.sid)) { + // A message with a new sid, but haven't seen the EOR for the + // previous message. Deliver the previous partial message to avoid + // merging messages from different sid's. + transport->invoker_.AsyncInvoke( + RTC_FROM_HERE, transport->network_thread_, + rtc::Bind(&SctpTransport::OnInboundPacketFromSctpToTransport, + transport, transport->partial_message_, + transport->partial_params_, transport->partial_flags_)); + + transport->partial_message_.Clear(); + } transport->partial_message_.AppendData(reinterpret_cast(data), length); - transport->partial_message_sid_ = rcv.rcv_sid; + transport->partial_params_ = params; + transport->partial_flags_ = flags; free(data); @@ -315,11 +332,6 @@ class SctpTransport::UsrSctpWrapper { return 1; } - params.sid = rcv.rcv_sid; - params.seq_num = rcv.rcv_ssn; - params.timestamp = rcv.rcv_tsn; - params.type = type; - // The ownership of the packet transfers to |invoker_|. Using // CopyOnWriteBuffer is the most convenient way to do this. transport->invoker_.AsyncInvoke( diff --git a/media/sctp/sctptransport.h b/media/sctp/sctptransport.h index 85c6627111..a401d8b795 100644 --- a/media/sctp/sctptransport.h +++ b/media/sctp/sctptransport.h @@ -146,7 +146,8 @@ class SctpTransport : public SctpTransportInternal, // Track the data received from usrsctp between callbacks until the EOR bit // arrives. rtc::CopyOnWriteBuffer partial_message_; - int partial_message_sid_; + ReceiveDataParams partial_params_; + int partial_flags_; bool was_ever_writable_ = false; int local_port_ = kSctpDefaultPort;