From 921ed4360ad64861d7782830f1df80c12fa5473d Mon Sep 17 00:00:00 2001 From: JackDoan Date: Mon, 27 Jul 2026 15:43:52 -0500 Subject: [PATCH] overlay/tio: validate WriteGSO geometry instead of silently dropping The length checks were fishy on four counts: an empty hdr/transportHdr with real payload returned nil (silent drop with a success signal); the HdrLen/GSOSize/CsumStart uint16 conversions could wrap unchecked; nothing verified transportHdr covers csum_start+csum_offset, so the kernel's NEEDS_CSUM write could land in payload bytes; and there was no total-size bound even though every length field involved is 16-bit. Malformed geometry is now a real error, and a single 65535 total-length guard makes all the u16 conversions exact. Co-Authored-By: Claude Fable 5 --- overlay/tio/tio_gso_linux.go | 25 +++++++++++++- overlay/tio/tun_linux_offload_test.go | 50 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/overlay/tio/tio_gso_linux.go b/overlay/tio/tio_gso_linux.go index c4762f54..b8361668 100644 --- a/overlay/tio/tio_gso_linux.go +++ b/overlay/tio/tio_gso_linux.go @@ -321,8 +321,15 @@ func (r *Offload) Capabilities() Capabilities { return Capabilities{TSO: true, USO: r.usoEnabled} } +// maxSuperpacketLen caps a WriteGSO superpacket (headers + payload). The +// virtio_net_hdr length fields and the IPv4 total-length / IPv6 +// payload-length stamped inside it are all 16-bit, so anything larger +// would wrap one of them and hand the kernel corrupt geometry. +const maxSuperpacketLen = 65535 + func (r *Offload) WriteGSO(hdr []byte, transportHdr []byte, pays [][]byte, proto GSOProto) error { - if len(hdr) == 0 || len(pays) == 0 || len(transportHdr) == 0 { + if len(pays) == 0 { + // No payload fragments at all: nothing to send. return nil } // L4 checksum offset inside transportHdr: TCP=16 (the `check` field after @@ -334,12 +341,23 @@ func (r *Offload) WriteGSO(hdr []byte, transportHdr []byte, pays [][]byte, proto default: csumOff = 16 } + // Malformed geometry must fail loudly, not vanish: the old empty-header + // early-out returned nil and silently dropped the payload. NEEDS_CSUM + // also makes the kernel write a checksum at csum_start+csum_offset, so + // transportHdr has to actually contain that field -- otherwise the + // write lands in payload bytes. + if len(hdr) == 0 || len(transportHdr) < int(csumOff)+2 { + return fmt.Errorf("tio: WriteGSO header too short: ip=%d transport=%d (csum field at %d)", + len(hdr), len(transportHdr), csumOff) + } // GSO geometry comes from the non-empty fragments only: the iovec loop // below skips empties, so gso_size must never be derived from one. A // leading empty fragment would otherwise stamp a superpacket header // with gso_size == 0, which the kernel rejects with EINVAL. segSize, segCount := 0, 0 + total := len(hdr) + len(transportHdr) for _, p := range pays { + total += len(p) if len(p) == 0 { continue } @@ -348,6 +366,11 @@ func (r *Offload) WriteGSO(hdr []byte, transportHdr []byte, pays [][]byte, proto } segCount++ } + // With total bounded, every uint16 conversion below (HdrLen, GSOSize, + // CsumStart) is exact. + if total > maxSuperpacketLen { + return fmt.Errorf("tio: WriteGSO superpacket %dB exceeds %d", total, maxSuperpacketLen) + } vhdr := virtio.Hdr{ Flags: unix.VIRTIO_NET_HDR_F_NEEDS_CSUM, HdrLen: uint16(len(hdr) + len(transportHdr)), diff --git a/overlay/tio/tun_linux_offload_test.go b/overlay/tio/tun_linux_offload_test.go index 7535b8f3..79ff3fb8 100644 --- a/overlay/tio/tun_linux_offload_test.go +++ b/overlay/tio/tun_linux_offload_test.go @@ -909,3 +909,53 @@ func TestWriteGSOLeadingEmptyFragmentGeometry(t *testing.T) { t.Errorf("wrote %d bytes want %d (empty fragment must not add an iovec)", n, want) } } + +// TestWriteGSORejectsBadGeometry pins the length-check contract: malformed +// 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 { + 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 + udpHdr := make([]byte, 8) + tcpHdr := make([]byte, 20) + seg := make([]byte, 1200) + + cases := []struct { + name string + hdr, thdr []byte + pays [][]byte + proto GSOProto + wantErr bool + }{ + {"empty-ip-hdr-with-payload", nil, udpHdr, [][]byte{seg}, GSOProtoUDP, true}, + {"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}, + {"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}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := o.WriteGSO(tc.hdr, tc.thdr, tc.pays, tc.proto) + if tc.wantErr && err == nil { + t.Errorf("WriteGSO = nil, want error") + } + if !tc.wantErr && err != nil { + t.Errorf("WriteGSO = %v, want nil", err) + } + }) + } +}