From 4d5139e7011d8d4facb94f5343aee328d8fc8e44 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 22 Aug 2026 04:55:23 -0400 Subject: [PATCH 1/2] checklocks: snapshot TCP send state atomically The send-buffer option callback atomically disables autotuning without holding the endpoint or send queue mutex. Endpoint probes copy that flag under the queue mutex, so their RacyLoad can race with the callback. 96459f55984 introduced the atomic flag alongside a non-atomic state snapshot; ef9e8d91318 preserved the race in CloneState. Load and store the snapshot flag atomically, declare its atomic contract and the queue lock precondition, and retain exclusive ownership of the destination. Extend the endpoint-probe test with a concurrent option callback to exercise the conflicting accesses through real packet processing. Assisted-by: Codex --- pkg/tcpip/transport/tcp/endpoint.go | 6 ++++-- pkg/tcpip/transport/tcp/state.go | 2 ++ pkg/tcpip/transport/tcp/test/e2e/tcp_test.go | 8 ++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index 79031b92cfd..58ce2d2ac8f 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -289,14 +289,16 @@ type sndQueueInfo struct { TCPSndBufState } -// CloneState clones sq into other. It is not thread safe +// CloneState clones sq into other, which must be exclusively owned by the caller. +// +// +checklocks:sq.sndQueueMu func (sq *sndQueueInfo) CloneState(other *TCPSndBufState) { other.SndBufSize = sq.SndBufSize other.SndBufUsed = sq.SndBufUsed other.SndClosed = sq.SndClosed other.PacketTooBigCount = sq.PacketTooBigCount other.SndMTU = sq.SndMTU - other.AutoTuneSndBufDisabled = atomicbitops.FromUint32(sq.AutoTuneSndBufDisabled.RacyLoad()) + other.AutoTuneSndBufDisabled.Store(sq.AutoTuneSndBufDisabled.Load()) } // Endpoint represents a TCP endpoint. This struct serves as the interface diff --git a/pkg/tcpip/transport/tcp/state.go b/pkg/tcpip/transport/tcp/state.go index d6ddd79c355..e1807978ddb 100644 --- a/pkg/tcpip/transport/tcp/state.go +++ b/pkg/tcpip/transport/tcp/state.go @@ -417,6 +417,8 @@ type TCPSndBufState struct { // AutoTuneSndBufDisabled indicates that the auto tuning of send buffer // is disabled. + // + // +checkatomic AutoTuneSndBufDisabled atomicbitops.Uint32 } diff --git a/pkg/tcpip/transport/tcp/test/e2e/tcp_test.go b/pkg/tcpip/transport/tcp/test/e2e/tcp_test.go index 21ed62a8049..c4ad3776e47 100644 --- a/pkg/tcpip/transport/tcp/test/e2e/tcp_test.go +++ b/pkg/tcpip/transport/tcp/test/e2e/tcp_test.go @@ -5798,6 +5798,14 @@ func TestTCPEndpointProbe(t *testing.T) { c.CreateConnected(context.TestInitialSequenceNumber, 30000, -1 /* epRcvBuf */) port = c.Port // c.Port is set during CreateConnected. + // The socket option callback can run concurrently with the probe without + // holding the endpoint or send queue mutex. + var wg sync.WaitGroup + defer wg.Wait() + wg.Go(func() { + _ = c.EP.(*tcp.Endpoint).OnSetSendBufferSize(4096) + }) + data := []byte{1, 2, 3} iss := seqnum.Value(context.TestInitialSequenceNumber).Add(1) c.SendPacket(data, &context.Headers{ From 364d9a863c3464d53516ffa59f29aaa38ee17062 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 22 Aug 2026 04:55:49 -0400 Subject: [PATCH 2/2] checklocks: preserve concurrent TCP close bits Readiness and shutdown can update the connection-direction cache under different mutexes. The load followed by swap added in 89b6a474c62 lets two updates read the same old value and overwrite each other, losing a send-closed or receive-closed bit. Use atomic OR to preserve concurrent updates and annotate the atomic field. No caller uses the previous value, so remove the helper result. Passing the zero-valued open state remains a no-op. Assisted-by: Codex --- pkg/tcpip/transport/tcp/endpoint.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index 58ce2d2ac8f..8190f1ee88e 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -411,8 +411,9 @@ type Endpoint struct { // methods. state atomicbitops.Uint32 `state:".(EndpointState)"` - // connectionDirectionState holds current state of send and receive, - // accessed atomically + // connectionDirectionState records whether sending and receiving are closed. + // + // +checkatomic connectionDirectionState atomicbitops.Uint32 // origEndpointState is only used during a restore phase to save the @@ -3097,14 +3098,15 @@ func (e *Endpoint) maxReceiveBufferSize() int { return rs.Max } -// directionState returns the close state of send and receive part of the endpoint +// connDirectionState returns the send and receive close state of the endpoint. func (e *Endpoint) connDirectionState() connDirectionState { return connDirectionState(e.connectionDirectionState.Load()) } -// updateDirectionState updates the close state of send and receive part of the endpoint -func (e *Endpoint) updateConnDirectionState(state connDirectionState) connDirectionState { - return connDirectionState(e.connectionDirectionState.Swap(uint32(e.connDirectionState() | state))) +// updateConnDirectionState adds closed directions to the endpoint's state. +// Passing connDirectionStateOpen leaves the state unchanged. +func (e *Endpoint) updateConnDirectionState(state connDirectionState) { + atomicbitops.OrUint32(&e.connectionDirectionState, uint32(state)) } // rcvWndScaleForHandshake computes the receive window scale to offer to the