Do not queue zero sized segments.

Commit 16b751b6c610ec2c5a913cb8a818e9239ee7da71 introduced a bug where writes of
zero size would end up queueing a zero sized segment which will cause the
sandbox to panic when trying to send a zero sized segment(e.g. after an RTO) as
netstack asserts that the all non FIN segments have size > 0.

This change adds the check for a zero sized payload back to avoid queueing
such segments. The associated test panics without the fix and passes with it.

PiperOrigin-RevId: 383677884

2 files changed, 38 insertions, 0 deletions

diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go
index 1ed4ba419..9945fdd6b 100644
--- a/pkg/tcpip/transport/tcp/endpoint.go
+++ b/pkg/tcpip/transport/tcp/endpoint.go
@@ -1530,6 +1530,12 @@ func (e *endpoint) queueSegment(p tcpip.Payloader, opts tcpip.WriteOptions) (*se
 	if err != nil {
 		return nil, 0, err
 	}
+
+	// Do not queue zero length segments.
+	if len(v) == 0 {
+		return nil, 0, nil
+	}
+
 	if !opts.Atomic {
 		// Since we released locks in between it's possible that the
 		// endpoint transitioned to a CLOSED/ERROR states so make
diff --git a/pkg/tcpip/transport/tcp/tcp_test.go b/pkg/tcpip/transport/tcp/tcp_test.go
index 71c4aa85d..3f67a1f1c 100644
--- a/pkg/tcpip/transport/tcp/tcp_test.go
+++ b/pkg/tcpip/transport/tcp/tcp_test.go
@@ -3624,6 +3624,38 @@ func TestMaxRTO(t *testing.T) {
 	}
 }
 
+// TestZeroSizedWriteRetransmit tests that a zero sized write should not
+// result in a panic on an RTO as no segment should have been queued for
+// a zero sized write.
+func TestZeroSizedWriteRetransmit(t *testing.T) {
+	c := context.New(t, defaultMTU)
+	defer c.Cleanup()
+
+	c.CreateConnected(context.TestInitialSequenceNumber, 30000 /* rcvWnd */, -1 /* epRcvBuf */)
+
+	var r bytes.Reader
+	_, err := c.EP.Write(&r, tcpip.WriteOptions{})
+	if err != nil {
+		t.Fatalf("Write failed: %s", err)
+	}
+	// Now do a non-zero sized write to trigger actual sending of data.
+	r.Reset(make([]byte, 1))
+	_, err = c.EP.Write(&r, tcpip.WriteOptions{})
+	if err != nil {
+		t.Fatalf("Write failed: %s", err)
+	}
+	// Do not ACK the packet and expect an original transmit and a
+	// retransmit. This should not cause a panic.
+	for i := 0; i < 2; i++ {
+		checker.IPv4(t, c.GetPacket(),
+			checker.TCP(
+				checker.DstPort(context.TestPort),
+				checker.TCPFlagsMatch(header.TCPFlagAck, ^header.TCPFlagPsh),
+			),
+		)
+	}
+}
+
 // TestRetransmitIPv4IDUniqueness tests that the IPv4 Identification field is
 // unique on retransmits.
 func TestRetransmitIPv4IDUniqueness(t *testing.T) {