From 7c9492c4b7b2173f82b634a328d59e1b40b7057c Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Wed, 20 May 2026 15:43:17 -0700 Subject: [PATCH] tun: clear virtioNetHdr when deleting item from tcpGROTable Otherwise the deleted packet gets written to the TUN fd with an invalid virtioNetHdr. Fixes tailscale/tailscale#19820 Signed-off-by: Jordan Whited --- tun/offload_linux.go | 22 ++++++------------ tun/offload_linux_test.go | 48 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/tun/offload_linux.go b/tun/offload_linux.go index fb6ac5b94..0d54479ee 100644 --- a/tun/offload_linux.go +++ b/tun/offload_linux.go @@ -623,6 +623,10 @@ func tcpGRO(bufs [][]byte, offset int, pktI int, table *tcpGROTable, isV6 bool) case coalesceItemInvalidCSum: // delete the item with an invalid csum table.deleteAt(item.key, i) + // The deleted item will not be re-visited in applyTCPCoalesceAccounting, + // so we must zero the virtioNetHdr. clear() is the equivalent of + // encoding a zero value virtioNetHdr. + clear(bufs[item.bufsIndex][offset-virtioNetHdrLen : offset]) case coalescePktInvalidCSum: // no point in inserting an item that we can't coalesce return groResultNoop @@ -682,11 +686,7 @@ func applyTCPCoalesceAccounting(bufs [][]byte, offset int, table *tcpGROTable) e psum := PseudoHeaderChecksum(unix.IPPROTO_TCP, srcAddr, dstAddr, uint16(len(pkt)-int(item.iphLen))) binary.BigEndian.PutUint16(pkt[hdr.csumStart+hdr.csumOffset:], Checksum([]byte{}, psum)) } else { - hdr := virtioNetHdr{} - err := hdr.encode(bufs[item.bufsIndex][offset-virtioNetHdrLen:]) - if err != nil { - return err - } + clear(bufs[item.bufsIndex][offset-virtioNetHdrLen : offset]) } } } @@ -742,11 +742,7 @@ func applyUDPCoalesceAccounting(bufs [][]byte, offset int, table *udpGROTable) e psum := PseudoHeaderChecksum(unix.IPPROTO_UDP, srcAddr, dstAddr, uint16(len(pkt)-int(item.iphLen))) binary.BigEndian.PutUint16(pkt[hdr.csumStart+hdr.csumOffset:], Checksum([]byte{}, psum)) } else { - hdr := virtioNetHdr{} - err := hdr.encode(bufs[item.bufsIndex][offset-virtioNetHdrLen:]) - if err != nil { - return err - } + clear(bufs[item.bufsIndex][offset-virtioNetHdrLen : offset]) } } } @@ -895,11 +891,7 @@ func handleGRO(bufs [][]byte, offset int, tcpTable *tcpGROTable, udpTable *udpGR } switch result { case groResultNoop: - hdr := virtioNetHdr{} - err := hdr.encode(bufs[i][offset-virtioNetHdrLen:]) - if err != nil { - return err - } + clear(bufs[i][offset-virtioNetHdrLen : offset]) fallthrough case groResultTableInsert: *toWrite = append(*toWrite, i) diff --git a/tun/offload_linux_test.go b/tun/offload_linux_test.go index 407037863..403e97a77 100644 --- a/tun/offload_linux_test.go +++ b/tun/offload_linux_test.go @@ -762,3 +762,51 @@ func Test_udpPacketsCanCoalesce(t *testing.T) { }) } } + +func Test_handleGRO_invalidItemCsumClearsVirtioNetHdr(t *testing.T) { + pkts := [][]byte{ + flipTCP4Checksum(tcp4Packet(ip4PortA, ip4PortB, header.TCPFlagAck, 100, 1)), + tcp4Packet(ip4PortA, ip4PortB, header.TCPFlagAck, 100, 101), + tcp4Packet(ip4PortA, ip4PortB, header.TCPFlagAck, 100, 201), + } + // Poison the virtioNetHdr region of pkts[0] so a missing clear() is detectable. + for i := 0; i < virtioNetHdrLen; i++ { + pkts[0][i] = 0xAB + } + + table := newTCPGROTable() + toWrite := make([]int, 0, len(pkts)) + if err := handleGRO(pkts, virtioNetHdrLen, table, newUDPGROTable(), 0, &toWrite); err != nil { + t.Fatal(err) + } + + // Verify pkts[0] is in toWrite where we expect + if toWrite[0] != 0 { + t.Fatal("pkts[0] not found in toWrite at expected, zero index") + } + + // Verify pkts[0] is not in tcpGROTable + if len(table.itemsByFlow) != 1 { + t.Fatalf("unexpected tcpGROTable items len: %d", len(table.itemsByFlow)) + } + for _, v := range table.itemsByFlow { + if len(v) != 1 { + t.Fatalf("unexpected tcpGROItems slice len: %d", len(v)) + } + item := v[0] + if item.sentSeq != 101 { + t.Fatalf("unexpected starting seq num in tcpGROTable: %d", item.sentSeq) + } + if item.numMerged != 1 { + t.Fatalf("unexpected numMerged in tcpGROTable: %d", item.numMerged) + } + } + + // pkt 0 is in toWrite and not present in tcpGROTable, so its virtioNetHdr + // must have been cleared. + for i, b := range pkts[0][:virtioNetHdrLen] { + if b != 0 { + t.Fatalf("pkts[0] virtioNetHdr[%d] = 0x%x, want 0", i, b) + } + } +}