Don't use uninitialized timestamp echo reply value.

[pkelsey]

Summary

If a TCP segment does not contain a timestamp option, the
auto receive buffer scaling code will take action based on an
uninitialized timestamp echo reply value off of the stack.

Check the timestamp option flag so this does not occur.

[hiren]

https://www.ietf.org/rfc/rfc1323.txt mentions:

The Timestamp Echo Reply field (TSecr) is only valid if the ACK
bit is set in the TCP header; if it is valid, it echos a times-
tamp value that was sent by the remote TCP in the TSval field
of a Timestamps option.  When TSecr is not valid, its value
must be zero.

Which I brought up to Patrick but then he mentioned that while following this and always initializing the tsecr field early in tcpopt processing would be the correct thing to do but the code is already established elsewhere to 1) check the option flag for the timestamp option and then 2) check if the value is not zero.

And I agree that because of that reasoning, the proposed fix seems sufficient to me.

Also adding network so someone else can also review this.

[pkelsey]

In D3060#60525, @hiren wrote:

https://www.ietf.org/rfc/rfc1323.txt mentions:

The Timestamp Echo Reply field (TSecr) is only valid if the ACK
bit is set in the TCP header; if it is valid, it echos a times-
tamp value that was sent by the remote TCP in the TSval field
of a Timestamps option.  When TSecr is not valid, its value
must be zero.

Which I brought up to Patrick but then he mentioned that while following this and always initializing the tsecr field early in tcpopt processing would be the correct thing to do but the code is already established elsewhere to 1) check the option flag for the timestamp option and then 2) check if the value is not zero.

And I agree that because of that reasoning, the proposed fix seems sufficient to me.

Also adding network so someone else can also review this.

Just to be clear (because it seems to me the above could be read to say we are not following the spec), regarding the quoted bit from the RFC, we are adhering to the requirement that ACK be set on the packet in order to have a valid tsecr field - we do not take any action based on tsecr if ACK is not set.

What I believed Hiren was asking me was whether we could address the issue I am patching here by ensuring to.to_tsecr is always initialized to zero prior to options processing. We could, as the code always treats to.to_tsecr == 0 as an invalid timestamp (because reasons, even though that condition does not imply an invalid timestamp per the spec), but the convention in all other existing to.to_tsecr handling code is to establish its validity by first testing that the TS option was set on the segment and second testing that to.to_tsecr is not zero, so for consistency I think adding the TS option flag check is the way to go here over the alternative to.to_tsecr initialization approach.