From cc0e96a4bd3348ecc2a636fd90118b8e5cce672c Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Thu, 28 Mar 2019 18:17:40 -0700 Subject: Fix Panic in SACKScoreboard.Delete. The panic was caused by modifying the tree while iterating which invalidated the iterator. Also fixes another bug in SACKScoreboard.Insert() which was causing blocks to be merged incorrectly. PiperOrigin-RevId: 240895053 Change-Id: Ia72b8244297962df5c04283346da5226434740af --- pkg/tcpip/transport/tcp/sack_scoreboard.go | 15 +++++++++++---- pkg/tcpip/transport/tcp/sack_scoreboard_test.go | 9 +++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/pkg/tcpip/transport/tcp/sack_scoreboard.go b/pkg/tcpip/transport/tcp/sack_scoreboard.go index 67f25f050..21878ad82 100644 --- a/pkg/tcpip/transport/tcp/sack_scoreboard.go +++ b/pkg/tcpip/transport/tcp/sack_scoreboard.go @@ -77,7 +77,7 @@ func (s *SACKScoreboard) Insert(r header.SACKBlock) { sacked := i.(header.SACKBlock) // There is a hole between these two SACK blocks, so we can't // merge anymore. - if r.End.LessThan(r.Start) { + if r.End.LessThan(sacked.Start) { return false } // There is some overlap at this point, merge the blocks and @@ -167,7 +167,11 @@ func (s *SACKScoreboard) String() string { // Delete removes all SACK information prior to seq. func (s *SACKScoreboard) Delete(seq seqnum.Value) { + if s.Empty() { + return + } toDelete := []btree.Item{} + toInsert := []btree.Item{} r := header.SACKBlock{seq, seq.Add(1)} s.ranges.DescendLessOrEqual(r, func(i btree.Item) bool { if i == r { @@ -179,13 +183,16 @@ func (s *SACKScoreboard) Delete(seq seqnum.Value) { s.sacked -= sb.Start.Size(sb.End) } else { newSB := header.SACKBlock{seq, sb.End} - s.ranges.ReplaceOrInsert(newSB) + toInsert = append(toInsert, newSB) s.sacked -= sb.Start.Size(seq) } return true }) - for _, i := range toDelete { - s.ranges.Delete(i) + for _, sb := range toDelete { + s.ranges.Delete(sb) + } + for _, sb := range toInsert { + s.ranges.ReplaceOrInsert(sb) } } diff --git a/pkg/tcpip/transport/tcp/sack_scoreboard_test.go b/pkg/tcpip/transport/tcp/sack_scoreboard_test.go index 1a1178999..3cf2ff451 100644 --- a/pkg/tcpip/transport/tcp/sack_scoreboard_test.go +++ b/pkg/tcpip/transport/tcp/sack_scoreboard_test.go @@ -77,6 +77,15 @@ func TestSACKScoreboardIsSACKED(t *testing.T) { }, 4294254144, }, + { + "Test disjoint SACKBlocks out of order", + []header.SACKBlock{{827450276, 827454536}, {827426028, 827428868}}, + []blockTest{ + {header.SACKBlock{827426028, 827428867}, true}, + {header.SACKBlock{827450168, 827450275}, false}, + }, + 827426000, + }, } for _, tc := range testCases { sb := initScoreboard(tc.scoreboardBlocks, tc.iss) -- cgit v1.2.3