tun: clear virtioNetHdr when deleting item from tcpGROTable

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)

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)
+		}
+	}
+}