Skip to content

Commit

Permalink
Ensure views returned by PullUp are owned exclusively by their packet.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 681977034
  • Loading branch information
manninglucas authored and gvisor-bot committed Oct 3, 2024
1 parent cceb04f commit a446b45
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 5 deletions.
7 changes: 5 additions & 2 deletions pkg/buffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,11 @@ func (b *Buffer) PullUp(offset, length int) (View, bool) {
if x := curr.Intersect(tgt); x.Len() == tgt.Len() {
// buf covers the whole requested target range.
sub := x.Offset(-curr.begin)
// Don't increment the reference count of the underlying chunk. Views
// returned by PullUp are explicitly unowned and read only
if v.sharesChunk() {
old := v.chunk
v.chunk = v.chunk.Clone()
old.DecRef()
}
new := View{
read: v.read + sub.begin,
write: v.read + sub.end,
Expand Down
4 changes: 3 additions & 1 deletion pkg/tcpip/network/ipv4/ipv4.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,9 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
return
}
}

// CheckPrerouting can modify the backing storage of the packet, so refresh
// the header.
h = header.IPv4(pkt.NetworkHeader().Slice())
e.handleValidatedPacket(h, pkt, e.nic.Name() /* inNICName */)
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/tcpip/network/ipv6/ipv6.go
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,9 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
}
}

// CheckPrerouting can modify the backing storage of the packet, so refresh
// the header.
h = header.IPv6(pkt.NetworkHeader().Slice())
e.handleValidatedPacket(h, pkt, e.nic.Name() /* inNICName */)
}

Expand Down Expand Up @@ -1495,6 +1498,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe
routerAlert *header.IPv6RouterAlertOption
)
for {
h := header.IPv6(pkt.NetworkHeader().Slice())
if done, err := e.processExtensionHeader(&it, &pkt, h, &routerAlert, &hasFragmentHeader, forwarding); err != nil || done {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/tcpip/transport/tcp/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func newIncomingSegment(id stack.TransportEndpointID, clock tcpip.Clock, pkt *st
s.window = seqnum.Size(hdr.WindowSize())
s.rcvdTime = clock.NowMonotonic()
s.dataMemSize = pkt.MemSize()
s.pkt = pkt.IncRef()
s.pkt = pkt.Clone()
s.csumValid = csumValid

if !s.pkt.RXChecksumValidated {
Expand Down
4 changes: 3 additions & 1 deletion pkg/tcpip/transport/udp/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,9 @@ func (e *endpoint) HandlePacket(id stack.TransportEndpointID, pkt *stack.PacketB
Addr: id.LocalAddress,
Port: hdr.DestinationPort(),
},
pkt: pkt.IncRef(),
// We need to clone the packet because ReadTo modifies the write index of
// the underlying buffer. Clone does not copy the data, just the metadata.
pkt: pkt.Clone(),
}
e.rcvList.PushBack(packet)
e.rcvBufSize += pkt.Data().Size()
Expand Down

0 comments on commit a446b45

Please sign in to comment.