mirror of
https://github.com/slackhq/nebula.git
synced 2026-08-16 12:26:58 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -321,8 +321,15 @@ func (r *Offload) Capabilities() Capabilities {
|
|||||||
return Capabilities{TSO: true, USO: r.usoEnabled}
|
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 {
|
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
|
return nil
|
||||||
}
|
}
|
||||||
// L4 checksum offset inside transportHdr: TCP=16 (the `check` field after
|
// 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:
|
default:
|
||||||
csumOff = 16
|
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
|
// 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
|
// below skips empties, so gso_size must never be derived from one. A
|
||||||
// leading empty fragment would otherwise stamp a superpacket header
|
// leading empty fragment would otherwise stamp a superpacket header
|
||||||
// with gso_size == 0, which the kernel rejects with EINVAL.
|
// with gso_size == 0, which the kernel rejects with EINVAL.
|
||||||
segSize, segCount := 0, 0
|
segSize, segCount := 0, 0
|
||||||
|
total := len(hdr) + len(transportHdr)
|
||||||
for _, p := range pays {
|
for _, p := range pays {
|
||||||
|
total += len(p)
|
||||||
if len(p) == 0 {
|
if len(p) == 0 {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -348,6 +366,11 @@ func (r *Offload) WriteGSO(hdr []byte, transportHdr []byte, pays [][]byte, proto
|
|||||||
}
|
}
|
||||||
segCount++
|
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{
|
vhdr := virtio.Hdr{
|
||||||
Flags: unix.VIRTIO_NET_HDR_F_NEEDS_CSUM,
|
Flags: unix.VIRTIO_NET_HDR_F_NEEDS_CSUM,
|
||||||
HdrLen: uint16(len(hdr) + len(transportHdr)),
|
HdrLen: uint16(len(hdr) + len(transportHdr)),
|
||||||
|
|||||||
@@ -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)
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user