Skip to content

Commit b47706c

Browse files
committed
net/transport: fix yamux dial fd-reuse race and outgoing sub-stream leak
H1: yamux Dial installed a net.Dialer.Control that, on ctx cancellation, called Shutdown+Close on the raw socket fd. DialContext(ctx) already aborts the connect and closes its own fd, so the two raced to close the same fd number (classic fd-reuse hazard). Drop the Control/abortOnCancel machinery and the abort_{unix,windows,other}.go files; rely on net.Dialer.Timeout + DialContext(ctx), which handle timeout and cancellation safely. H2: peer.openDrpcConn opened a sub-stream then ran OutgoingProtoHandshake, returning the error without closing the stream. The handshake does not close the conn on three protocol-level error paths (incompatible/declined/ unexpected proto), so a version-skewed peer leaked streams unboundedly. Close the sub-stream on any handshake error, matching the incoming serve() path. Refs GO-7313
1 parent 8b582a6 commit b47706c

5 files changed

Lines changed: 11 additions & 111 deletions

File tree

net/peer/peer.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,11 @@ func (p *peer) openDrpcConn(ctx context.Context) (*subConn, error) {
275275
lastUsageConn := connutil.NewLastUsageConn(conn)
276276
proto, err := handshake.OutgoingProtoHandshake(ctx, lastUsageConn, defaultHandshakeProto)
277277
if err != nil {
278+
// OutgoingProtoHandshake closes the conn on I/O errors and ctx
279+
// cancellation, but returns without closing on some protocol-level
280+
// errors (incompatible/declined/unexpected proto). Close here so the
281+
// sub-stream we opened above never leaks. Double close is harmless.
282+
_ = lastUsageConn.Close()
278283
return nil, err
279284
}
280285
bufSize := p.ctrl.DrpcConfig().Stream.MaxMsgSizeMb * (1 << 20)

net/transport/yamux/abort_other.go

Lines changed: 0 additions & 26 deletions
This file was deleted.

net/transport/yamux/abort_unix.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

net/transport/yamux/abort_windows.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

net/transport/yamux/yamux.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,24 +104,19 @@ func (y *yamuxTransport) AddListener(lis net.Listener) {
104104

105105
func (y *yamuxTransport) Dial(ctx context.Context, addr string) (mc transport.MultiConn, err error) {
106106
dialTimeout := time.Duration(y.conf.DialTimeoutSec) * time.Second
107-
108-
// Setup abort mechanism for context cancellation
109-
done := make(chan struct{})
110-
107+
// net.Dialer.Timeout together with DialContext(ctx) already aborts the TCP
108+
// connect on timeout or ctx cancellation and closes the underlying fd
109+
// through the runtime. We must not abort the socket ourselves: closing the
110+
// raw fd that the netpoller owns races with that cleanup and can close an
111+
// unrelated, reused descriptor.
111112
dialer := &net.Dialer{
112113
Timeout: dialTimeout,
113-
Control: controlFunc(ctx, done),
114114
}
115-
116115
conn, err := dialer.DialContext(ctx, "tcp", addr)
117-
118-
// Signal the abortor goroutine to exit
119-
close(done)
120-
121116
if err != nil {
122117
return nil, err
123118
}
124-
119+
125120
ctx, cancel := context.WithTimeout(ctx, dialTimeout)
126121
defer cancel()
127122
cctx, err := y.secure.SecureOutbound(ctx, conn)

0 commit comments

Comments
 (0)