tio.Offload.WriteGSO: reject 0-len packets, check seg lengths

overlay/tio/tio.go

@@ -115,7 +115,7 @@ const (
 // (mutable: the L4 checksum field must hold the pseudo-header partial, single-fold not inverted, per virtio NEEDS_CSUM semantics).
 // pays are non-overlapping payload fragments whose concatenation is the full superpacket payload.
 // They are read-only from the writer's perspective and must remain valid until the call returns.
-// Every segment in pays except possibly the last is exactly the same size.
+// Every segment in pays except possibly the last must be exactly the same size.
 // proto picks the L4 protocol so the writer knows which gsoType / CsumOffset to set.
 //
 // Callers should also consult CapsProvider (via SupportsGSO) for the per-protocol negotiated capability:

overlay/tio/tio_gso_linux.go

@@ -327,52 +327,45 @@ func (r *Offload) WriteGSO(hdr []byte, transportHdr []byte, pays [][]byte, proto
 	r.gsoIovs[2].Base = &transportHdr[0]
 	r.gsoIovs[2].SetLen(len(transportHdr))
 
-	// Fill out the payload iovecs and find the GSO geometry:
+	segSize := len(pays[0])
-	segSize := 0
 	total := len(hdr) + len(transportHdr)
-	n := 3
+	for i, p := range pays {
-	for _, p := range pays {
-		total += len(p)
 		if len(p) == 0 {
-			continue //disregard empty payloads, the kernel will reject them.
+			// The coalescers route zero-payload packets down the non-GSO path,
-			//callers already funnel zero-length frames via the not-GSO path, so this should never happen.
+			// so an empty fragment means the caller's accounting is broken.
+			return fmt.Errorf("tio: WriteGSO empty payload fragment %d of %d", i, len(pays))
+		} else if len(p) > segSize || (len(p) < segSize && i != len(pays)-1) {
+			// all segments must be the same size, except for the last one
+			return fmt.Errorf("tio: WriteGSO fragment %d is %dB, want %dB segments (only the last may be shorter)", i, len(p), segSize)
 		}
-		if n == 3 {
+		total += len(p)
-			segSize = len(p)
+		r.gsoIovs[3+i].Base = &p[0]
+		r.gsoIovs[3+i].SetLen(len(p))
 	}
-		r.gsoIovs[n].Base = &p[0]
-		r.gsoIovs[n].SetLen(len(p))
-		n++
-	}
-	r.gsoIovs = r.gsoIovs[:n]
-	segCount := n - 3
 	// This check keeps `total` in the uint16 range. Anything larger would wrap around and cause trouble.
 	if total > maxSuperpacketLen {
 		return fmt.Errorf("tio: WriteGSO superpacket %dB exceeds %d", total, maxSuperpacketLen)
 	}
-	// gsoType and GSOSize stay zero (GSO_NONE, 0) for single-segment, or an unknown IP version.
+
+	// A single segment ships as a plain checksummed packet (GSO_NONE, size 0).
+	// Multiple segments carry the real GSO type and segSize, which the loop
+	// above verified is the size of every fragment except possibly the last.
+	gsoType := uint8(unix.VIRTIO_NET_HDR_GSO_NONE)
+	if len(pays) > 1 {
+		gsoType = gsoTypeFromProto(proto, hdr[0]>>4)
+	}
+	var gsoSize uint16
+	if gsoType != unix.VIRTIO_NET_HDR_GSO_NONE {
+		gsoSize = uint16(segSize)
+	}
 	vhdr := virtio.NewHeader(
 		unix.VIRTIO_NET_HDR_F_NEEDS_CSUM,   /*flags*/
-		unix.VIRTIO_NET_HDR_GSO_NONE,       /*gsoType*/
+		gsoType,                            /*gsoType*/
 		uint16(len(hdr)+len(transportHdr)), /*hdrLen*/
-		0,                                  /*gsoSize*/
+		gsoSize,                            /*gsoSize*/
 		uint16(len(hdr)),                   /*csumStart*/
 		csumOff,                            /*csumOffset*/
 	)
-	if segCount > 1 {
-		ipVer := hdr[0] >> 4
-		switch {
-		case proto == GSOProtoUDP && (ipVer == 4 || ipVer == 6):
-			vhdr.SetGSOType(unix.VIRTIO_NET_HDR_GSO_UDP_L4)
-		case ipVer == 6:
-			vhdr.SetGSOType(unix.VIRTIO_NET_HDR_GSO_TCPV6)
-		case ipVer == 4:
-			vhdr.SetGSOType(unix.VIRTIO_NET_HDR_GSO_TCPV4)
-		}
-		if vhdr.GSOType() != unix.VIRTIO_NET_HDR_GSO_NONE {
-			vhdr.GSOSize = uint16(segSize)
-		}
-	}
 	vhdr.Encode(r.gsoHdrBuf[:])
 
 	_, err := r.rawWrite(r.gsoIovs)

overlay/tio/tun_linux_offload.go

@@ -25,6 +25,20 @@ func protoFromGSOType(t uint8) (GSOProto, error) {
 	}
 }
 
+// gsoTypeFromProto is the reverse of protoFromGSOType
+func gsoTypeFromProto(proto GSOProto, ipVer uint8) uint8 {
+	switch {
+	case proto == GSOProtoUDP && (ipVer == 4 || ipVer == 6):
+		return unix.VIRTIO_NET_HDR_GSO_UDP_L4
+	case ipVer == 6:
+		return unix.VIRTIO_NET_HDR_GSO_TCPV6
+	case ipVer == 4:
+		return unix.VIRTIO_NET_HDR_GSO_TCPV4
+	default:
+		return unix.VIRTIO_NET_HDR_GSO_NONE
+	}
+}
+
 // SegmentSuperpacket invokes fn once per segment of pkt.
 // For non-GSO pkts fn is called once with pkt.Bytes.
 // For GSO/USO superpackets, fn is called once per segment with a slice of pkt.Bytes holding that segment's plaintext

overlay/tio/tun_linux_offload_test.go

@@ -744,38 +744,6 @@ func TestSegmentSuperpacketNoAlloc(t *testing.T) {
 	}
 }
 
-// TestWriteGSOSkipsEmptyPayloads is the defense-in-depth guard for the
-// zero-length UDP DoS: a payload fragment of length zero would make &p[0]
-// panic (index-out-of-range) when building the iovec array. WriteGSO must
-// skip empties instead. We write to /dev/null so the writev always succeeds
-// synchronously; the point is simply that neither call panics.
-func TestWriteGSOSkipsEmptyPayloads(t *testing.T) {
-	fd, err := unix.Open("/dev/null", os.O_WRONLY, 0)
-	if err != nil {
-		t.Fatalf("open /dev/null: %v", err)
-	}
-	t.Cleanup(func() { _ = unix.Close(fd) })
-
-	o := &Offload{fd: fd, gsoIovs: make([]unix.Iovec, 2, gsoMaxIovs)}
-	o.gsoIovs[0].Base = &o.gsoHdrBuf[0]
-	o.gsoIovs[0].SetLen(virtio.Size)
-
-	ipHdr := make([]byte, 20)
-	ipHdr[0] = 0x45 // IPv4, IHL 5
-	udpHdr := make([]byte, 8)
-
-	// Sole payload empty: exercises the all-empty skip (n stays at 3).
-	if err := o.WriteGSO(ipHdr, udpHdr, [][]byte{{}}, GSOProtoUDP); err != nil {
-		t.Fatalf("WriteGSO with a single empty payload: %v", err)
-	}
-	// Empty mixed with a real fragment: exercises the index-drift skip so a
-	// later non-empty payload still lands in the right iovec slot.
-	real := make([]byte, 1200)
-	if err := o.WriteGSO(ipHdr, udpHdr, [][]byte{real, {}}, GSOProtoUDP); err != nil {
-		t.Fatalf("WriteGSO with a trailing empty payload: %v", err)
-	}
-}
-
 // buildTSOv6 builds a synthetic IPv6/TCP TSO superpacket with payLen bytes
 // of payload, segmented at gso. Returns the packet bytes only; the
 // virtio_net_hdr is the caller's responsibility.
@@ -943,12 +911,12 @@ func TestOffloadWriteZeroLength(t *testing.T) {
 	}
 }
 
-// TestWriteGSOLeadingEmptyFragmentGeometry: gso_size must be derived from
+// TestWriteGSOSuperpacketGeometry decodes the vnet header the kernel would see for a multi-segment write:
-// the first non-empty fragment. Deriving it from pays[0] stamped a
+// the GSO type must match the proto and IP version
-// superpacket header with gso_size == 0 when the leading fragment was
+// gso_size must be the per-segment size (the kernel rejects a superpacket with gso_size == 0),
-// empty -- the kernel rejects that with EINVAL and the burst is lost.
+// and the csum fields must point at the transport header's checksum slot.
-// Write through a pipe and decode the vnet header the kernel would see.
+// Write through a pipe so the bytes can be read back and decoded.
-func TestWriteGSOLeadingEmptyFragmentGeometry(t *testing.T) {
+func TestWriteGSOSuperpacketGeometry(t *testing.T) {
 	var pfds [2]int
 	if err := unix.Pipe(pfds[:]); err != nil {
 		t.Fatalf("pipe: %v", err)
@@ -964,8 +932,8 @@ func TestWriteGSOLeadingEmptyFragmentGeometry(t *testing.T) {
 	udpHdr := make([]byte, 8)
 	seg := make([]byte, 1200)
 
-	if err := o.WriteGSO(ipHdr, udpHdr, [][]byte{{}, seg, seg}, GSOProtoUDP); err != nil {
+	if err := o.WriteGSO(ipHdr, udpHdr, [][]byte{seg, seg}, GSOProtoUDP); err != nil {
-		t.Fatalf("WriteGSO with leading empty fragment: %v", err)
+		t.Fatalf("WriteGSO: %v", err)
 	}
 
 	buf := make([]byte, virtio.Size+len(ipHdr)+len(udpHdr)+2*len(seg)+64)
@@ -979,18 +947,20 @@ func TestWriteGSOLeadingEmptyFragmentGeometry(t *testing.T) {
 		t.Errorf("gsoType=%d want UDP_L4", vhdr.GSOType())
 	}
 	if vhdr.GSOSize != 1200 {
-		t.Errorf("GSOSize=%d want 1200 (first non-empty fragment)", vhdr.GSOSize)
+		t.Errorf("GSOSize=%d want 1200 (per-segment size from pays[0])", vhdr.GSOSize)
+	}
+	if vhdr.HdrLen != uint16(len(ipHdr)+len(udpHdr)) {
+		t.Errorf("HdrLen=%d want %d", vhdr.HdrLen, len(ipHdr)+len(udpHdr))
+	}
+	if vhdr.CsumStart != uint16(len(ipHdr)) || vhdr.CsumOffset != 6 {
+		t.Errorf("csum start/offset = %d/%d want %d/6", vhdr.CsumStart, vhdr.CsumOffset, len(ipHdr))
 	}
 	if want := virtio.Size + len(ipHdr) + len(udpHdr) + 2*len(seg); n != want {
-		t.Errorf("wrote %d bytes want %d (empty fragment must not add an iovec)", n, want)
+		t.Errorf("wrote %d bytes want %d", n, want)
 	}
 }
 
-// TestWriteGSORejectsBadGeometry pins the length-check contract: malformed
+// TestWriteGSORejectsBadGeometry pins the length-check contracts
-// geometry must fail loudly instead of silently succeeding (the old empty-
-// header early-out returned nil and dropped the payload), and nothing may
-// reach the u16 virtio fields or the kernel's csum_start+csum_offset write
-// without covering them.
 func TestWriteGSORejectsBadGeometry(t *testing.T) {
 	fd, err := unix.Open("/dev/null", os.O_WRONLY, 0)
 	if err != nil {
@@ -1019,6 +989,13 @@ func TestWriteGSORejectsBadGeometry(t *testing.T) {
 		{"udp-transport-too-short-for-csum", ipHdr, udpHdr[:6], [][]byte{seg}, GSOProtoUDP, true},
 		{"tcp-transport-too-short-for-csum", ipHdr, tcpHdr[:16], [][]byte{seg}, GSOProtoTCP, true},
 		{"superpacket-over-65535", ipHdr, tcpHdr, [][]byte{make([]byte, 40000), make([]byte, 40000)}, GSOProtoTCP, true},
+		{"sole-payload-empty", ipHdr, udpHdr, [][]byte{{}}, GSOProtoUDP, true},
+		{"leading-empty-fragment", ipHdr, udpHdr, [][]byte{{}, seg, seg}, GSOProtoUDP, true},
+		{"trailing-empty-fragment", ipHdr, tcpHdr, [][]byte{seg, {}}, GSOProtoTCP, true},
+		{"oversize-middle-fragment", ipHdr, udpHdr, [][]byte{seg, make([]byte, 1201), seg}, GSOProtoUDP, true},
+		{"undersize-middle-fragment", ipHdr, udpHdr, [][]byte{seg, make([]byte, 100), seg}, GSOProtoUDP, true},
+		{"oversize-last-fragment", ipHdr, tcpHdr, [][]byte{seg, make([]byte, 1201)}, GSOProtoTCP, true},
+		{"short-last-fragment-ok", ipHdr, udpHdr, [][]byte{seg, seg, make([]byte, 100)}, GSOProtoUDP, false},
 		{"no-pays-noop", ipHdr, udpHdr, nil, GSOProtoUDP, false},
 		{"valid-udp", ipHdr, udpHdr, [][]byte{seg, seg}, GSOProtoUDP, false},
 		{"valid-tcp", ipHdr, tcpHdr, [][]byte{seg, seg}, GSOProtoTCP, false},