Replace most instances of IncRef with Clone.

Incrementing the reference count of a packet as a means of granting ownership
is unsafe when the packet is shared across gorountines. The underlying buffer's
reference count is unchanged since it "technically" has the same owning
PacketBuffer, which means different goroutines operating on the underlying
buffer (and packet itself) race.

Clones are roughly as fast as IncRefs because the PacketBuffers allocate from
a pool and the underlying buffers are cloned with copy-on-write
semantics.

I've left IncRef in places where the original packet in obviously going out of
scope at the end of the function or in some tests.

Reported-by: syzbot+e026046f4bf8ad09ae1f@syzkaller.appspotmail.com
Reported-by: syzbot+559365d6050db4b30e0f@syzkaller.appspotmail.com
Reported-by: syzbot+63c78a2c88a5744c636b@syzkaller.appspotmail.com
PiperOrigin-RevId: 705676806

pkg/tcpip/link/channel/channel.go

@@ -86,11 +86,12 @@ func (q *queue) Write(pkt *stack.PacketBuffer) tcpip.Error {
 	}
 
 	wrote := false
+	p := pkt.Clone()
 	select {
-	case q.c <- pkt.IncRef():
+	case q.c <- p:
 		wrote = true
 	default:
-		pkt.DecRef()
+		p.DecRef()
 	}
 	notify := q.notify
 	q.mu.RUnlock()

pkg/tcpip/link/fdbased/endpoint_test.go

@@ -133,8 +133,7 @@ func (c *testContext) cleanup() {
 }
 
 func (c *testContext) DeliverNetworkPacket(protocol tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) {
-	pkt.IncRef()
-	c.ch <- packetInfo{protocol, pkt}
+	c.ch <- packetInfo{protocol, pkt.Clone()}
 }
 
 func (c *testContext) DeliverLinkPacket(tcpip.NetworkProtocolNumber, *stack.PacketBuffer) {
@@ -597,8 +596,7 @@ type fakeNetworkDispatcher struct {
 }
 
 func (d *fakeNetworkDispatcher) DeliverNetworkPacket(_ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) {
-	pkt.IncRef()
-	d.pkts = append(d.pkts, pkt)
+	d.pkts = append(d.pkts, pkt.Clone())
 }
 
 func (*fakeNetworkDispatcher) DeliverLinkPacket(tcpip.NetworkProtocolNumber, *stack.PacketBuffer) {

pkg/tcpip/link/fdbased/processors.go

@@ -167,8 +167,7 @@ func (m *processorManager) queuePacket(pkt *stack.PacketBuffer, hasEthHeader boo
 	p := &m.processors[pIdx]
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	pkt.IncRef()
-	p.pkts.PushBack(pkt)
+	p.pkts.PushBack(pkt.IncRef())
 	m.ready[pIdx] = true
 }
 

pkg/tcpip/link/veth/veth_test.go

@@ -49,8 +49,7 @@ type testNetworkDispatcher struct {
 }
 
 func (d *testNetworkDispatcher) DeliverNetworkPacket(_ tcpip.NetworkProtocolNumber, pkt *stack.PacketBuffer) {
-	pkt.IncRef()
-	d.ch <- pkt
+	d.ch <- pkt.Clone()
 	d.wg.Wait()
 }
 

pkg/tcpip/network/internal/fragmentation/fragmentation_test.go

@@ -582,7 +582,9 @@ type testTimeoutHandler struct {
 }
 
 func (h *testTimeoutHandler) OnReassemblyTimeout(pkt *stack.PacketBuffer) {
-	h.pkt = pkt
+	if pkt != nil {
+		h.pkt = pkt.Clone()
+	}
 }
 
 func TestTimeoutHandler(t *testing.T) {
@@ -673,6 +675,11 @@ func TestTimeoutHandler(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			handler := &testTimeoutHandler{pkt: nil}
+			defer func() {
+				if handler.pkt != nil {
+					handler.pkt.DecRef()
+				}
+			}()
 
 			f := NewFragmentation(minBlockSize, HighFragThreshold, LowFragThreshold, reassembleTimeout, &faketime.NullClock{}, handler)
 

pkg/tcpip/network/internal/fragmentation/reassembler.go

@@ -135,7 +135,7 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt *s
 			last:   last,
 			filled: true,
 			final:  currentHole.final,
-			pkt:    pkt.IncRef(),
+			pkt:    pkt.Clone(),
 		}
 		r.filled++
 		// For IPv6, it is possible to have different Protocol values between
@@ -150,7 +150,7 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt *s
 			if r.pkt != nil {
 				r.pkt.DecRef()
 			}
-			r.pkt = pkt.IncRef()
+			r.pkt = pkt.Clone()
 			r.proto = proto
 		}
 		break

pkg/tcpip/network/internal/testutil/testutil.go

@@ -79,7 +79,7 @@ func (ep *MockLinkEndpoint) WritePackets(pkts stack.PacketBufferList) (int, tcpi
 			return n, ep.err
 		}
 		ep.allowPackets--
-		ep.WrittenPackets = append(ep.WrittenPackets, pkt.IncRef())
+		ep.WrittenPackets = append(ep.WrittenPackets, pkt.Clone())
 		n++
 	}
 	return n, nil

pkg/tcpip/stack/forwarding_test.go

@@ -319,7 +319,7 @@ func (e *fwdTestLinkEndpoint) WritePackets(pkts PacketBufferList) (int, tcpip.Er
 	n := 0
 	for _, pkt := range pkts.AsSlice() {
 		select {
-		case e.C <- pkt:
+		case e.C <- pkt.IncRef():
 		default:
 		}
 

pkg/tcpip/stack/packet_buffer.go

@@ -381,6 +381,7 @@ func (pk *PacketBuffer) Clone() *PacketBuffer {
 	newPk.Hash = pk.Hash
 	newPk.Owner = pk.Owner
 	newPk.GSOOptions = pk.GSOOptions
+	newPk.EgressRoute = pk.EgressRoute
 	newPk.NetworkProtocolNumber = pk.NetworkProtocolNumber
 	newPk.dnatDone = pk.dnatDone
 	newPk.snatDone = pk.snatDone

pkg/tcpip/stack/pending_packets.go

@@ -149,7 +149,7 @@ func (f *packetsPendingLinkResolution) enqueue(r *Route, pkt *PacketBuffer) tcpi
 	packets, ok := f.mu.packets[ch]
 	packets = append(packets, pendingPacket{
 		routeInfo: routeInfo,
-		pkt:       pkt.IncRef(),
+		pkt:       pkt.Clone(),
 	})
 
 	if len(packets) > maxPendingPacketsPerResolution {

pkg/tcpip/transport/udp/forwarder.go

@@ -47,7 +47,7 @@ func (f *Forwarder) HandlePacket(id stack.TransportEndpointID, pkt *stack.Packet
 	f.handler(&ForwarderRequest{
 		stack: f.stack,
 		id:    id,
-		pkt:   pkt.IncRef(),
+		pkt:   pkt.Clone(),
 	})
 
 	return true