From 3f7a9f82e39813ca42701aca18cf23a97b5652dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Claus=20Lensb=C3=B8l?= <claus@tailscale.com>
Date: Fri, 6 Jun 2025 11:42:33 -0400
Subject: [PATCH] wgengine/magicsock: fix bpf fragmentation jump offsets
 (#16204)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fragmented datagrams would be processed instead of being dumped right
away. In reality, thse datagrams would be dropped anyway later so there
should functionally not be any change. Additionally, the feature is off
by default.

Closes #16203

Signed-off-by: Claus Lensbøl <claus@tailscale.com>
---
 wgengine/magicsock/magicsock_linux.go      |  4 +-
 wgengine/magicsock/magicsock_linux_test.go | 76 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/wgengine/magicsock/magicsock_linux.go b/wgengine/magicsock/magicsock_linux.go
index c5df555cd..34c39fe62 100644
--- a/wgengine/magicsock/magicsock_linux.go
+++ b/wgengine/magicsock/magicsock_linux.go
@@ -66,10 +66,10 @@ var (
 		// fragmented, and we don't want to handle reassembly.
 		bpf.LoadAbsolute{Off: 6, Size: 2},
 		// More Fragments bit set means this is part of a fragmented packet.
-		bpf.JumpIf{Cond: bpf.JumpBitsSet, Val: 0x2000, SkipTrue: 7, SkipFalse: 0},
+		bpf.JumpIf{Cond: bpf.JumpBitsSet, Val: 0x2000, SkipTrue: 8, SkipFalse: 0},
 		// Non-zero fragment offset with MF=0 means this is the last
 		// fragment of packet.
-		bpf.JumpIf{Cond: bpf.JumpBitsSet, Val: 0x1fff, SkipTrue: 6, SkipFalse: 0},
+		bpf.JumpIf{Cond: bpf.JumpBitsSet, Val: 0x1fff, SkipTrue: 7, SkipFalse: 0},
 
 		// Load IP header length into X register.
 		bpf.LoadMemShift{Off: 0},
diff --git a/wgengine/magicsock/magicsock_linux_test.go b/wgengine/magicsock/magicsock_linux_test.go
index 6b86b04f2..28ccd220e 100644
--- a/wgengine/magicsock/magicsock_linux_test.go
+++ b/wgengine/magicsock/magicsock_linux_test.go
@@ -9,6 +9,7 @@ import (
 	"net/netip"
 	"testing"
 
+	"golang.org/x/net/bpf"
 	"golang.org/x/sys/cpu"
 	"golang.org/x/sys/unix"
 	"tailscale.com/disco"
@@ -146,3 +147,78 @@ func TestEthernetProto(t *testing.T) {
 		}
 	}
 }
+
+func TestBpfDiscardV4(t *testing.T) {
+	// Good packet as a reference for what should not be rejected
+	udp4Packet := []byte{
+		// IPv4 header
+		0x45, 0x00, 0x00, 0x26, 0x00, 0x00, 0x00, 0x00,
+		0x40, 0x11, 0x00, 0x00,
+		0x7f, 0x00, 0x00, 0x01, // source ip
+		0x7f, 0x00, 0x00, 0x02, // dest ip
+
+		// UDP header
+		0x30, 0x39, // src port
+		0xd4, 0x31, // dest port
+		0x00, 0x12, // length; 8 bytes header + 10 bytes payload = 18 bytes
+		0x00, 0x00, // checksum; unused
+
+		// Payload: disco magic plus 32 bytes for key and 24 bytes for nonce
+		0x54, 0x53, 0xf0, 0x9f, 0x92, 0xac, 0x00, 0x01, 0x02, 0x03,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00,
+	}
+
+	vm, err := bpf.NewVM(magicsockFilterV4)
+	if err != nil {
+		t.Fatalf("failed creating BPF VM: %v", err)
+	}
+
+	tests := []struct {
+		name    string
+		replace map[int]byte
+		accept  bool
+	}{
+		{
+			name:    "base accepted datagram",
+			replace: map[int]byte{},
+			accept:  true,
+		},
+		{
+			name: "more fragments",
+			replace: map[int]byte{
+				6: 0x20,
+			},
+			accept: false,
+		},
+		{
+			name: "some fragment",
+			replace: map[int]byte{
+				7: 0x01,
+			},
+			accept: false,
+		},
+	}
+
+	udp4PacketChanged := make([]byte, len(udp4Packet))
+	copy(udp4PacketChanged, udp4Packet)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			for k, v := range tt.replace {
+				udp4PacketChanged[k] = v
+			}
+			ret, err := vm.Run(udp4PacketChanged)
+			if err != nil {
+				t.Fatalf("BPF VM error: %v", err)
+			}
+
+			if (ret != 0) != tt.accept {
+				t.Errorf("expected accept=%v, got ret=%v", tt.accept, ret)
+			}
+		})
+	}
+}