zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dcsctp: Abandon chunks marked for fast retransmit

Browse Source 
The current code assumed that chunks that were scheduled for fast
retransmission would never be abandoned, as chunks marked for fast
retransmission would be immediately sent after the SACK has been
processed, giving no time for them to be abandoned.

But fuzzers keep on fuzzing, and can craft a sequence of chunks that
result in a SACK that both marks the chunks for fast retransmission and
later (while processing the same SACK) abandons them.

Bug: chromium:1331087
Change-Id: Id218607e18a6f3a9d6d51044dccb920e1e77372a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264960
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37108}
This commit is contained in:
Victor Boivie
2022-06-02 21:49:12 +02:00
committed by WebRTC LUCI CQ
parent 05225514e6
commit 7726b7d067
2 changed files with 46 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
net/dcsctp/tx/outstanding_data.cc
View File

@ -284,8 +284,7 @@ void OutstandingData::AbandonAllFor(const Item& item) {
RTC_DLOG(LS_VERBOSE) << "Marking chunk " << *tsn.Wrap()
<< " as abandoned";
if (other.should_be_retransmitted()) {
RTC_DCHECK(to_be_fast_retransmitted_.find(tsn) ==
to_be_fast_retransmitted_.end());
to_be_fast_retransmitted_.erase(tsn);
to_be_retransmitted_.erase(tsn);
}
other.Abandon();

45
net/dcsctp/tx/outstanding_data_test.cc
View File

@ -474,5 +474,50 @@ TEST_F(OutstandingDataTest, MustRetransmitBeforeGettingNackedAgain) {
EXPECT_FALSE(buf_.has_data_to_be_retransmitted());
}
TEST_F(OutstandingDataTest, CanAbandonChunksMarkedForFastRetransmit) {
// This test is a bit convoluted, and can't really happen with a well behaving
// client, but this was found by fuzzers. This test will verify that a message
// that was both marked as "to be fast retransmitted" and "abandoned" at the
// same time doesn't cause any consistency issues.
// Add chunks 10-14, but chunk 11 has zero retransmissions. When chunk 10 and
// 11 are NACKed three times, chunk 10 will be marked for retransmission, but
// chunk 11 will be abandoned, which also abandons chunk 10, as it's part of
// the same message.
buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow,
TimeMs::InfiniteFuture()); // 10
buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits(0), kNow,
TimeMs::InfiniteFuture()); // 11
buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow,
TimeMs::InfiniteFuture()); // 12
buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow,
TimeMs::InfiniteFuture()); // 13
buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow,
TimeMs::InfiniteFuture()); // 13
// ACK 9, 12
std::vector<SackChunk::GapAckBlock> gab1 = {SackChunk::GapAckBlock(3, 3)};
EXPECT_FALSE(
buf_.HandleSack(unwrapper_.Unwrap(TSN(9)), gab1, false).has_packet_loss);
EXPECT_FALSE(buf_.has_data_to_be_retransmitted());
// ACK 9, 12, 13
std::vector<SackChunk::GapAckBlock> gab2 = {SackChunk::GapAckBlock(3, 4)};
EXPECT_FALSE(
buf_.HandleSack(unwrapper_.Unwrap(TSN(9)), gab2, false).has_packet_loss);
EXPECT_FALSE(buf_.has_data_to_be_retransmitted());
EXPECT_CALL(on_discard_, Call(IsUnordered(false), StreamID(1), MID(42)))
.WillOnce(Return(false));
// ACK 9, 12, 13, 14
std::vector<SackChunk::GapAckBlock> gab3 = {SackChunk::GapAckBlock(3, 5)};
OutstandingData::AckInfo ack =
buf_.HandleSack(unwrapper_.Unwrap(TSN(9)), gab3, false);
EXPECT_TRUE(ack.has_packet_loss);
EXPECT_FALSE(buf_.has_data_to_be_retransmitted());
EXPECT_THAT(buf_.GetChunksToBeFastRetransmitted(1000), IsEmpty());
EXPECT_THAT(buf_.GetChunksToBeRetransmitted(1000), IsEmpty());
}
} // namespace
} // namespace dcsctp