summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorBhasker Hariharan <bhaskerh@google.com>2019-03-28 18:17:40 -0700
committerShentubot <shentubot@google.com>2019-03-28 18:18:39 -0700
commitcc0e96a4bd3348ecc2a636fd90118b8e5cce672c (patch)
tree6e87e6ea5eabab3a4509fbfde447fbd677fb13bf
parent31c2236e97db6a57ca9b6ab3771876cd2231fd85 (diff)
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
-rw-r--r--pkg/tcpip/transport/tcp/sack_scoreboard.go15
-rw-r--r--pkg/tcpip/transport/tcp/sack_scoreboard_test.go9
2 files changed, 20 insertions, 4 deletions
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)