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

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)