tcp: mitigate illegal state transitions on simultaneous close #279
Conversation
If a socket is closed and the remote side sends a RST before the local side sends its own RST, it is possible to jump directly from FIN-WAIT-1 to CLOSING without ever sending a RST to the remote side. The main side effect is that if untransmitted data is present, the socket is never exhausted, causing `EthernetInterface::poll` to loop forever. The ideal fix is to follow RFC more closely and not transition to FIN-WAIT-1 until the RST has been sent, but there are too many assumptions based on the current state that would be broken by remaining in ESTABLISHED with a closed transmit buffer to be worth fixing a relatively rare edge case. Instead, add another illegal transition to fix the failures of a previous violation, in the spirit of all good TCP stacks.
|
I acknowledge the issue, but I'll need to think about the fix. |
|
Can you tell me a bit more about this fix? In particular you are talking about RST in the PR description, but I can't find how your code or tests use RST at all. Do you mean FIN? |
|
Oof, that’s a dumb error. Please s/RST/FIN |
|
Thank you for identifying this issue! This is definitely a bug in the state machine that needs fixing. I believe the fix is incorrect though. Tracking whether we've sent a FIN is not enough: it may get lost, so we may have to retransmit it. Therefore we need to send FINs in the CLOSING state itself. Enabling the existing code to transmit FINs for CLOSING (in addition to FIN-WAIT-1 and LAST-ACK) fixes the issue both for the initial FIN and the retransmissions. To speed things up I've opened #398 with that fix, so I'm closing this. |
If a socket is closed and the remote side sends a FIN before the local
side sends its own FIN, it is possible to jump directly from FIN-WAIT-1
to CLOSING without ever sending a FIN to the remote side.
The main side effect is that if untransmitted data is present, the
socket is never exhausted, causing
EthernetInterface::pollto loopforever.
The ideal fix is to follow RFC more closely and not transition to
FIN-WAIT-1 until the FIN has been sent, but there are too many
assumptions based on the current state that would be broken by remaining
in ESTABLISHED with a closed transmit buffer to be worth fixing
a relatively rare edge case. Instead, add another illegal transition to
fix the failures of a previous violation, in the spirit of all good TCP
stacks.
The main reason why this is a problem at all is because
pollperforms ingress before egress, which is how this possibility can even arise. I haven't thought too hard about the implications of swapping them around. Maybe because, as already stated, this is a pretty rare occurrence to begin with.