diff options
author | Ian Gudger <igudger@google.com> | 2020-02-27 14:14:34 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-02-27 14:15:44 -0800 |
commit | c6bdc6b05b4abfcb3c677013496c9a6b1d1365dd (patch) | |
tree | 5ec78044281d27c9f47b8e04916ae0204d834a56 /pkg/tcpip/transport/tcp/endpoint.go | |
parent | d9ee81183fee2288681822a179ef230226fb8930 (diff) |
Fix a race in TCP endpoint teardown and teardown the stack in tcp_test.
Call stack.Close on stacks when we are done with them in tcp_test. This avoids
leaking resources and reduces the test's flakiness when race/gotsan is enabled.
It also provides test coverage for the race also fixed in this change, which
can be reliably triggered with the stack.Close change (and without the other
changes) when race/gotsan is enabled.
The race was possible when calling Abort (via stack.Close) on an endpoint
processing a SYN segment as part of a passive connect.
Updates #1564
PiperOrigin-RevId: 297685432
Diffstat (limited to 'pkg/tcpip/transport/tcp/endpoint.go')
-rw-r--r-- | pkg/tcpip/transport/tcp/endpoint.go | 16 |
1 files changed, 15 insertions, 1 deletions
diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index f1ad19dac..9e72730bd 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -798,7 +798,21 @@ func (e *endpoint) Abort() { // If the endpoint disconnected after the check, nothing needs to be // done, so sending a notification which will potentially be ignored is // fine. - if e.EndpointState().connected() { + // + // If the endpoint connecting finishes after the check, the endpoint + // is either in a connected state (where we would notifyAbort anyway), + // SYN-RECV (where we would also notifyAbort anyway), or in an error + // state where nothing is required and the notification can be safely + // ignored. + // + // Endpoints where a Close during connecting or SYN-RECV state would be + // problematic are set to state connecting before being registered (and + // thus possible to be Aborted). They are never available in initial + // state. + // + // Endpoints transitioning from initial to connecting state may be + // safely either closed or sent notifyAbort. + if s := e.EndpointState(); s == StateConnecting || s == StateSynRecv || s.connected() { e.notifyProtocolGoroutine(notifyAbort) return } |