Requested Comment/Message wording changes

PiperOrigin-RevId: 342366891

3 files changed, 17 insertions, 10 deletions

diff --git a/pkg/tcpip/header/ipv4.go b/pkg/tcpip/header/ipv4.go
index 713d476d6..91fe7b6a5 100644
--- a/pkg/tcpip/header/ipv4.go
+++ b/pkg/tcpip/header/ipv4.go
@@ -89,7 +89,17 @@ type IPv4Fields struct {
 	// DstAddr is the "destination ip address" of an IPv4 packet.
 	DstAddr tcpip.Address
 
-	// Options is between 0 and 40 bytes or nil if empty.
+	// Options must be 40 bytes or less as they must fit along with the
+	// rest of the IPv4 header into the maximum size describable in the
+	// IHL field. RFC 791 section 3.1 says:
+	//    IHL:  4 bits
+	//
+	//    Internet Header Length is the length of the internet header in 32
+	//    bit words, and thus points to the beginning of the data.  Note that
+	//    the minimum value for a correct header is 5.
+	//
+	// That leaves ten 32 bit (4 byte) fields for options. An attempt to encode
+	// more will fail.
 	Options IPv4Options
 }
 
@@ -284,13 +294,10 @@ func (o IPv4Options) SizeWithPadding() int {
 	return (len(o) + IPv4IHLStride - 1) & ^(IPv4IHLStride - 1)
 }
 
-// Options returns a buffer holding the options or nil.
+// Options returns a buffer holding the options.
 func (b IPv4) Options() IPv4Options {
 	hdrLen := b.HeaderLength()
-	if hdrLen > IPv4MinimumSize {
-		return IPv4Options(b[options:hdrLen:hdrLen])
-	}
-	return nil
+	return IPv4Options(b[options:hdrLen:hdrLen])
 }
 
 // TransportProtocol implements Network.TransportProtocol.
diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go
index 58a19e74a..204b182e6 100644
--- a/pkg/tcpip/network/ipv4/icmp.go
+++ b/pkg/tcpip/network/ipv4/icmp.go
@@ -90,7 +90,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 
 	iph := header.IPv4(pkt.NetworkHeader().View())
 	var newOptions header.IPv4Options
-	if len(iph) > header.IPv4MinimumSize {
+	if opts := iph.Options(); len(opts) != 0 {
 		// RFC 1122 section 3.2.2.6 (page 43) (and similar for other round trip
 		// type ICMP packets):
 		//    If a Record Route and/or Time Stamp option is received in an
@@ -106,7 +106,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 		} else {
 			op = &optionUsageReceive{}
 		}
-		aux, tmp, err := e.processIPOptions(pkt, iph.Options(), op)
+		aux, tmp, err := e.processIPOptions(pkt, opts, op)
 		if err != nil {
 			switch {
 			case
diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index b4f21d61e..a9a38b851 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -713,11 +713,11 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 		e.handleICMP(pkt)
 		return
 	}
-	if len(h.Options()) != 0 {
+	if opts := h.Options(); len(opts) != 0 {
 		// TODO(gvisor.dev/issue/4586):
 		// When we add forwarding support we should use the verified options
 		// rather than just throwing them away.
-		aux, _, err := e.processIPOptions(pkt, h.Options(), &optionUsageReceive{})
+		aux, _, err := e.processIPOptions(pkt, opts, &optionUsageReceive{})
 		if err != nil {
 			switch {
 			case