From aad7fca9c504effdaf3c77dd635e85c94dc4521d Mon Sep 17 00:00:00 2001
From: Jordan Whited <jordan@tailscale.com>
Date: Fri, 24 Mar 2023 15:09:47 -0700
Subject: [PATCH] tun: disqualify tcp4 packets w/IP options from coalescing

IP options were not being compared prior to coalescing. They are not
commonly used. Disqualification due to nonzero options is in line with
the kernel.

Reviewed-by: Denton Gentry <dgentry@tailscale.com>
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tun/tcp_offload_linux.go      | 10 +++----
 tun/tcp_offload_linux_test.go | 50 +++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/tun/tcp_offload_linux.go b/tun/tcp_offload_linux.go
index e807f00..4912efd 100644
--- a/tun/tcp_offload_linux.go
+++ b/tun/tcp_offload_linux.go
@@ -397,9 +397,6 @@ func tcpGRO(bufs [][]byte, offset int, pktI int, table *tcpGROTable, isV6 bool)
 		if totalLen != len(pkt) {
 			return false
 		}
-		if iphLen < 20 || iphLen > 60 {
-			return false
-		}
 	}
 	if len(pkt) < iphLen {
 		return false
@@ -474,13 +471,16 @@ func tcpGRO(bufs [][]byte, offset int, pktI int, table *tcpGROTable, isV6 bool)
 	return false
 }
 
-func isTCP4(b []byte) bool {
+func isTCP4NoIPOptions(b []byte) bool {
 	if len(b) < 40 {
 		return false
 	}
 	if b[0]>>4 != 4 {
 		return false
 	}
+	if b[0]&0x0F != 5 {
+		return false
+	}
 	if b[9] != unix.IPPROTO_TCP {
 		return false
 	}
@@ -511,7 +511,7 @@ func handleGRO(bufs [][]byte, offset int, tcp4Table, tcp6Table *tcpGROTable, toW
 		}
 		var coalesced bool
 		switch {
-		case isTCP4(bufs[i][offset:]):
+		case isTCP4NoIPOptions(bufs[i][offset:]): // ipv4 packets w/IP options do not coalesce
 			coalesced = tcpGRO(bufs, offset, i, tcp4Table, false)
 		case isTCP6NoEH(bufs[i][offset:]): // ipv6 packets w/extension headers do not coalesce
 			coalesced = tcpGRO(bufs, offset, i, tcp6Table, true)
diff --git a/tun/tcp_offload_linux_test.go b/tun/tcp_offload_linux_test.go
index 11f9e53..046e177 100644
--- a/tun/tcp_offload_linux_test.go
+++ b/tun/tcp_offload_linux_test.go
@@ -271,3 +271,53 @@ func Test_handleGRO(t *testing.T) {
 		})
 	}
 }
+
+func Test_isTCP4NoIPOptions(t *testing.T) {
+	valid := tcp4Packet(ip4PortA, ip4PortB, header.TCPFlagAck, 100, 1)[virtioNetHdrLen:]
+	invalidLen := valid[:39]
+	invalidHeaderLen := make([]byte, len(valid))
+	copy(invalidHeaderLen, valid)
+	invalidHeaderLen[0] = 0x46
+	invalidProtocol := make([]byte, len(valid))
+	copy(invalidProtocol, valid)
+	invalidProtocol[9] = unix.IPPROTO_TCP + 1
+
+	tests := []struct {
+		name string
+		b    []byte
+		want bool
+	}{
+		{
+			"valid",
+			valid,
+			true,
+		},
+		{
+			"invalid length",
+			invalidLen,
+			false,
+		},
+		{
+			"invalid version",
+			[]byte{0x00},
+			false,
+		},
+		{
+			"invalid header len",
+			invalidHeaderLen,
+			false,
+		},
+		{
+			"invalid protocol",
+			invalidProtocol,
+			false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isTCP4NoIPOptions(tt.b); got != tt.want {
+				t.Errorf("isTCP4NoIPOptions() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}