tcp: guard handleConnecting against a zeroed handshake (sing-box-lx SPEC 048)
listenContext.performHandshake, in its failure branch, zeroes ep.h and releases ep.mu before calling ep.Close(); the endpoint state only changes later, inside closeLocked. In that window the endpoint is still SynSent/SynRecv, so connecting() is true. A segment arriving there wakes the dispatcher, which routes to handleConnecting. Its gate checks the state but not h, so ep.h.processSegments() runs on a nil handshake and panics with a nil receiver at connect.go:534 — killing the whole process, not just the connection. The two conditions used to coincide; they stopped coinciding once zeroing h moved ahead of Close(). Fix: bail out early when ep.h is nil, releasing the mutex the same way the existing state gate does. One guard covers all five ep.h dereferences in handleConnecting (processSegments, listenEP in the error branch, and both inside deliverAccepted, which is only reached from here). Trigger in the field: TCP that never reaches established (silent server, RST, timeout) while SYN retransmits keep arriving. Reproduced from a device crash bundle (sing-box 1.14.0-lx.19-rc.3, android/arm64) where both sides of the race hold the same endpoint address; the accompanying test recreates that window and panics without this guard. SPECS/TASKS/048-GVISOR_HANDSHAKE_NIL_CRASH
This commit is contained in:
parent
2c4ae3b0a4
commit
ffebe42860
2 changed files with 57 additions and 0 deletions
|
|
@ -149,6 +149,18 @@ func handleConnecting(ep *Endpoint) {
|
|||
ep.mu.Unlock()
|
||||
return
|
||||
}
|
||||
// lx:begin handshake-nil-guard (sing-box-lx SPECS/TASKS/048)
|
||||
// listenContext.performHandshake zeroes ep.h and releases ep.mu before
|
||||
// calling ep.Close(); the state only changes later, inside closeLocked.
|
||||
// A segment arriving in that window wakes handleConnecting, whose gate
|
||||
// above checks the state but not h, so every ep.h dereference below —
|
||||
// processSegments, listenEP in the error branch, and deliverAccepted —
|
||||
// runs on a nil handshake and takes down the whole process.
|
||||
if ep.h == nil {
|
||||
ep.mu.Unlock()
|
||||
return
|
||||
}
|
||||
// lx:end handshake-nil-guard
|
||||
if err := ep.h.processSegments(); err != nil { // +checklocksforce:ep.h.ep.mu
|
||||
// handshake failed. clean up the tcp endpoint and handshake
|
||||
// state.
|
||||
|
|
|
|||
45
pkg/tcpip/transport/tcp/handshake_nil_guard_lx_test.go
Normal file
45
pkg/tcpip/transport/tcp/handshake_nil_guard_lx_test.go
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
// lx:begin handshake-nil-guard (sing-box-lx SPECS/TASKS/048)
|
||||
|
||||
package tcp
|
||||
|
||||
import "testing"
|
||||
|
||||
// TestHandleConnectingWithNilHandshake pins the guard in handleConnecting.
|
||||
//
|
||||
// listenContext.performHandshake, in its failure branch (accept.go), does:
|
||||
//
|
||||
// ep.mu.Lock()
|
||||
// ep.h = nil // handshake destroyed
|
||||
// ep.mu.Unlock() // mutex released — state is still SynSent/SynRecv
|
||||
// ep.Close() // state only changes here, inside closeLocked
|
||||
//
|
||||
// A segment arriving between the Unlock and Close wakes the dispatcher, which
|
||||
// dispatches to handleConnecting because connecting() is still true. Without
|
||||
// the guard, ep.h.processSegments() dereferences a nil handshake and panics
|
||||
// with a nil receiver at connect.go — taking down the entire process.
|
||||
//
|
||||
// This test recreates that window exactly: no stubbing, the state passes the
|
||||
// real connecting() gate and control reaches the dereference on its own.
|
||||
//
|
||||
// Without the guard this test panics; with it, the segment is a silent no-op.
|
||||
func TestHandleConnectingWithNilHandshake(t *testing.T) {
|
||||
ep := &Endpoint{}
|
||||
ep.state.Store(uint32(StateSynRecv))
|
||||
ep.h = nil
|
||||
|
||||
if !ep.EndpointState().connecting() {
|
||||
t.Fatalf("precondition failed: state %v is not connecting", ep.EndpointState())
|
||||
}
|
||||
|
||||
// What processor.start does for an endpoint with a queued segment.
|
||||
handleConnecting(ep)
|
||||
|
||||
// The guard must leave the mutex unlocked for the closing side, which is
|
||||
// blocked on LockUser inside ep.Close().
|
||||
if !ep.TryLock() {
|
||||
t.Fatal("handleConnecting returned holding ep.mu — Close() would deadlock")
|
||||
}
|
||||
ep.mu.Unlock()
|
||||
}
|
||||
|
||||
// lx:end handshake-nil-guard
|
||||
Loading…
Add table
Add a link
Reference in a new issue