mirror of
https://github.com/slackhq/nebula.git
synced 2026-08-15 08:07:03 +02:00
datapath: fix 12 correctness findings from tun/UDP offload review
Multi-disciplinary correctness review of the batched tun / GSO-GRO / sendmmsg rework. Each fix has a regression test; the merged tree builds on linux/darwin/openbsd/windows/freebsd/netbsd, vets clean, passes the unit and e2e suites, and is -race clean. Critical: - C1 zero-length inner UDP datagram no longer panics the process (remote DoS): the UDP coalescer routes payLen==0 to passthrough instead of seeding a GSO slot, and WriteGSO skips empty payload iovecs as defense in depth. - C2 segmenter no longer corrupts inner headers when gsoSize < headerLen: the L3+L4 header is snapshotted once and each segment stamped from the copy, replacing the destructive overlapping in-place slide (SegmentTCP + SegmentUDP). High: - H1 applyOuterECN updates the IPv4 header checksum (RFC 1624 incremental) when folding outer CE into the inner ToS, so passthrough packets are no longer dropped by the peer stack. - H2 the GRO reject path caps the borrowed RX segment ([:n:n]) so a reject can no longer overrun into the next coalesced segment's Nebula header. Note: oversized ICMPv6 rejects that need >16B beyond the segment are now refused rather than sent under GRO (safe; see TOFIX.md for the scratch-buffer follow-up). - H3 WriteBatch falls back to per-packet WriteTo for a chunk when writeSockaddr fails, so one bad-family destination costs only its own packet, not the batch. - H4 UserDevice.Readers returns N distinct queue wrappers with private buffers (sharing the pipes) so concurrent readers no longer race/overwrite borrowed packet bytes. - H5 Poll.Close / Offload.Close no longer null t.fd (matching master's tunFile.Close), removing the data race with a concurrent readOne load. Medium/Low: - M1 the UDP GSO 127-segment gate moved from kernel >=5.5 to >=6.9 (the real UDP_MAX_SEGMENTS 64->128 threshold), avoiding EINVAL + per-packet fallback on 5.5-6.8 kernels. - M2 NewMultiQueueReader replays the offload mask newTun actually negotiated instead of the TSO-only mask, so adding a queue no longer disables USO device-wide; the advertised USO capability derives from the same mask. - M3 the shutdown eventfd is closed in pollQueueSet.Close / offloadQueueSet.Close (double-close guarded), fixing the per-lifecycle fd leak. - M4 dual-stack ECN selects the cmsg by address family, not socket family: RX parseRecvCmsg reads both IP_TOS and IPV6_TCLASS; TX writeEntryCmsg stamps IP_TOS for v4/v4-mapped dests and IPV6_TCLASS for v6 (on-host verified). - L1 newPoll no longer closes the fd on failure (matching newOffload), removing the double-close on QueueSet.Add error.
This commit is contained in:
@@ -152,6 +152,17 @@ func (c *UDPCoalescer) commitParsed(pkt []byte, info parsedUDP) error {
|
||||
c.addPassthrough(pkt)
|
||||
return nil
|
||||
}
|
||||
// A zero-length UDP datagram (UDP `length` == 8) is legal and must still
|
||||
// reach the TUN, but it can't be coalesced: a GSO slot would store an
|
||||
// empty payload iovec and the kernel has nothing to segment. Seal any
|
||||
// open chain for this flow (so a later, non-empty datagram seeds fresh
|
||||
// *after* this one and per-flow arrival order is preserved) and deliver
|
||||
// it as a plain single datagram.
|
||||
if info.payLen == 0 {
|
||||
delete(c.openSlots, info.fk)
|
||||
c.addPassthrough(pkt)
|
||||
return nil
|
||||
}
|
||||
if open := c.openSlots[info.fk]; open != nil {
|
||||
if c.canAppend(open, pkt, info) {
|
||||
c.appendPayload(open, pkt, info)
|
||||
|
||||
@@ -365,6 +365,74 @@ func TestUDPCoalescerFragmentedIPv4PassesThrough(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A zero-length UDP datagram (UDP length == 8, no payload) is legal and
|
||||
// must be delivered as a plain single datagram — never coalesced. Seeding
|
||||
// it into a GSO slot stores an empty payload iovec that panics WriteGSO
|
||||
// (index-out-of-range on &pay[0]); this is a remote DoS if we ever let it
|
||||
// reach the GSO path. Regression: must not panic and must be written.
|
||||
func TestUDPCoalescerZeroLengthPayloadPassesThrough(t *testing.T) {
|
||||
w := &fakeTunWriter{gsoEnabled: true}
|
||||
c := NewUDPCoalescer(w, NewArena(0))
|
||||
pkt := buildUDPv4(1000, 53, nil) // UDP length 8, zero payload
|
||||
if err := c.Commit(pkt); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := c.Flush(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(w.writes) != 1 || len(w.gsoWrites) != 0 {
|
||||
t.Fatalf("zero-length UDP must pass through plain, got writes=%d gso=%d", len(w.writes), len(w.gsoWrites))
|
||||
}
|
||||
if len(w.writes[0]) != len(pkt) {
|
||||
t.Errorf("delivered %d bytes, want the whole %d-byte datagram", len(w.writes[0]), len(pkt))
|
||||
}
|
||||
}
|
||||
|
||||
// IPv6 zero-length UDP datagram: same passthrough contract as v4.
|
||||
func TestUDPCoalescerZeroLengthPayloadIPv6PassesThrough(t *testing.T) {
|
||||
w := &fakeTunWriter{gsoEnabled: true}
|
||||
c := NewUDPCoalescer(w, NewArena(0))
|
||||
pkt := buildUDPv6(1000, 53, nil) // UDP length 8, zero payload
|
||||
if err := c.Commit(pkt); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := c.Flush(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(w.writes) != 1 || len(w.gsoWrites) != 0 {
|
||||
t.Fatalf("zero-length IPv6 UDP must pass through plain, got writes=%d gso=%d", len(w.writes), len(w.gsoWrites))
|
||||
}
|
||||
if len(w.writes[0]) != len(pkt) {
|
||||
t.Errorf("delivered %d bytes, want the whole %d-byte datagram", len(w.writes[0]), len(pkt))
|
||||
}
|
||||
}
|
||||
|
||||
// A zero-length datagram arriving mid-flow must seal the open chain so the
|
||||
// datagram after it seeds a fresh superpacket *after* the empty one on the
|
||||
// wire — per-flow arrival order (full, empty, full) must be preserved.
|
||||
func TestUDPCoalescerZeroLengthMidFlowSealsAndPreservesOrder(t *testing.T) {
|
||||
w := &fakeTunWriter{gsoEnabled: true}
|
||||
c := NewUDPCoalescer(w, NewArena(0))
|
||||
full := make([]byte, 800)
|
||||
if err := c.Commit(buildUDPv4(1000, 53, full)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := c.Commit(buildUDPv4(1000, 53, nil)); err != nil { // zero-length
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := c.Commit(buildUDPv4(1000, 53, full)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := c.Flush(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The empty datagram sealed the first slot, so the trailing full packet
|
||||
// can't join it: two single-segment superpackets bracket one plain write.
|
||||
if len(w.gsoWrites) != 2 || len(w.writes) != 1 {
|
||||
t.Fatalf("want 2 gso writes + 1 plain, got gso=%d plain=%d", len(w.gsoWrites), len(w.writes))
|
||||
}
|
||||
}
|
||||
|
||||
// IPv4 with options is not admissible (we require IHL=5).
|
||||
func TestUDPCoalescerIPv4WithOptionsPassesThrough(t *testing.T) {
|
||||
w := &fakeTunWriter{gsoEnabled: true}
|
||||
|
||||
Reference in New Issue
Block a user