From bd58900fba9ea5278622d3331ddccb60f3ecac97 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Mon, 8 Jul 2024 17:46:00 -0700 Subject: [PATCH] Change veth WritePackets to deep clone its list of packets. The buffers from the buffer package are meant to be copy-on-write, but IP headers still work with the underlying []byte, so writes are not tracked. This allows data races in situations where packet buffers are shared across goroutines. PiperOrigin-RevId: 650429239 --- pkg/tcpip/link/veth/veth.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/tcpip/link/veth/veth.go b/pkg/tcpip/link/veth/veth.go index 8f96ebd4a..06d48e2be 100644 --- a/pkg/tcpip/link/veth/veth.go +++ b/pkg/tcpip/link/veth/veth.go @@ -208,10 +208,16 @@ func (e *Endpoint) WritePackets(pkts stack.PacketBufferList) (int, tcpip.Error) for _, pkt := range pkts.AsSlice() { // In order to properly loop back to the inbound side we must create a // fresh packet that only contains the underlying payload with no headers - // or struct fields set. + // or struct fields set. We must deep clone the payload to avoid + // two goroutines writing to the same buffer. + // + // TODO(b/240580913): Remove this once IP headers use reference counted + // views instead of raw byte slices. + payload := pkt.ToBuffer() newPkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ - Payload: pkt.ToBuffer(), + Payload: payload.DeepClone(), }) + payload.Release() (e.veth.backlogQueue) <- vethPacket{ e: e.peer, protocol: pkt.NetworkProtocolNumber,