From e739f982c087bc28b30ca6c707e5d653f5667b14 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Mon, 27 Dec 2021 15:41:37 -0800 Subject: [PATCH] Don't overwrite original IP header for Echo reply ...to avoid data races with other goroutines that may read from the same byte slice. This can occur when a packet is delivered to a raw or packet endpoint and we modify the IP header (to send the reply) at the the same time when another goroutine handles a read operation for a raw or packet endpoint. PiperOrigin-RevId: 418534467 --- pkg/tcpip/network/ipv4/icmp.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index ce8ec96ab..92f5aa6f0 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -262,11 +262,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { return } - // Take the base of the incoming request IP header but replace the options. - replyHeaderLength := uint8(header.IPv4MinimumSize + len(newOptions)) - replyIPHdr := header.IPv4(append(iph[:header.IPv4MinimumSize:header.IPv4MinimumSize], newOptions...)) - replyIPHdr.SetHeaderLength(replyHeaderLength) - // As per RFC 1122 section 3.2.1.3, when a host sends any datagram, the IP // source address MUST be one of its own IP addresses (but not a broadcast // or multicast address). @@ -313,6 +308,14 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { // We need to produce the entire packet in the data segment in order to // use WriteHeaderIncludedPacket(). WriteHeaderIncludedPacket sets the // total length and the header checksum so we don't need to set those here. + // + // Take the base of the incoming request IP header but replace the options. + replyHeaderLength := uint8(header.IPv4MinimumSize + len(newOptions)) + replyIPHdrBytes := make([]byte, 0, replyHeaderLength) + replyIPHdrBytes = append(replyIPHdrBytes, iph[:header.IPv4MinimumSize]...) + replyIPHdrBytes = append(replyIPHdrBytes, newOptions...) + replyIPHdr := header.IPv4(replyIPHdrBytes) + replyIPHdr.SetHeaderLength(replyHeaderLength) replyIPHdr.SetSourceAddress(r.LocalAddress()) replyIPHdr.SetDestinationAddress(r.RemoteAddress()) replyIPHdr.SetTTL(r.DefaultTTL())