Fix CurrentConnected counter overflow

The CurrentConnected counter is decremented in multiple places and in function
`maybeFailTimerHandler`, the caller does not make sure the state was one of the
connected states, causing overflow. This change puts the decrement logic in a
single place in `setEndpointState`. Added a test that would fail before this
change, though it is not the only case an overflow can happen.

https://fxbug.dev/105542

PiperOrigin-RevId: 464938313
This commit is contained in:
Zeling Feng
2022-08-02 18:34:04 -07:00
committed by gVisor bot
parent 87f4e4a188
commit ac4e0320bd
4 changed files with 34 additions and 4 deletions
-2
View File
@@ -130,7 +130,6 @@ func maybeFailTimerHandler(e *endpoint, f func() tcpip.Error) func() {
e.lastError = err
e.lastErrorMu.Unlock()
e.hardError = err
e.stack.Stats().TCP.CurrentConnected.Decrement()
e.cleanupLocked()
e.setEndpointState(StateError)
e.mu.Unlock()
@@ -1050,7 +1049,6 @@ func (e *endpoint) transitionToStateCloseLocked() {
}
if s.connected() {
e.stack.Stats().TCP.CurrentConnected.Decrement()
e.stack.Stats().TCP.EstablishedClosed.Increment()
}
-1
View File
@@ -196,7 +196,6 @@ func (p *processor) handleConnected(ep *endpoint) {
fallthrough
case ep.EndpointState() == StateClose:
ep.mu.Unlock()
ep.stack.Stats().TCP.CurrentConnected.Decrement()
ep.drainClosingSegmentQueue()
ep.waiterQueue.Notify(waiter.EventHUp | waiter.EventErr | waiter.ReadableEvents | waiter.WritableEvents)
return
+3 -1
View File
@@ -731,6 +731,9 @@ func (e *endpoint) setEndpointState(state EndpointState) {
if oldstate == StateCloseWait || oldstate == StateEstablished {
e.stack.Stats().TCP.EstablishedResets.Increment()
}
if oldstate.connected() {
e.stack.Stats().TCP.CurrentConnected.Decrement()
}
fallthrough
default:
if oldstate == StateEstablished {
@@ -977,7 +980,6 @@ func (e *endpoint) Abort() {
// Reset all connected endpoints.
switch state := e.EndpointState(); {
case state.connected():
e.stack.Stats().TCP.CurrentConnected.Decrement()
e.resetConnectionLocked(&tcpip.ErrAborted{})
return
}
@@ -292,6 +292,37 @@ func TestCloseWithoutConnect(t *testing.T) {
}
}
func TestHandshakeTimeoutConnectedCount(t *testing.T) {
c := context.New(t, e2e.DefaultMTU)
defer c.Cleanup()
ep, err := c.Stack().NewEndpoint(tcp.ProtocolNumber, ipv4.ProtocolNumber, &c.WQ)
if err != nil {
t.Fatalf("NewEndpoint failed: %s", err)
}
c.EP = ep
we, ch := waiter.NewChannelEntry(waiter.WritableEvents)
c.WQ.EventRegister(&we)
defer c.WQ.EventUnregister(&we)
switch err := c.EP.Connect(tcpip.FullAddress{Addr: context.TestAddr, Port: context.TestPort}).(type) {
case *tcpip.ErrConnectStarted:
default:
t.Fatalf("Connect did not start: %v", err)
}
<-ch
switch err := c.EP.LastError().(type) {
case *tcpip.ErrTimeout:
default:
t.Fatalf("Connect didn't timeout: %v", err)
}
if got, want := c.Stack().Stats().TCP.CurrentConnected.Value(), uint64(0); got != want {
t.Fatalf("got stats.TCP.CurrentConnected.Value() = %d, want = %d", got, want)
}
}
func TestTCPSegmentsSentIncrement(t *testing.T) {
c := context.New(t, e2e.DefaultMTU)
defer c.Cleanup()