From 4c4d20b72c2ffcc65325634d57021774dd2a1750 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 27 May 2026 11:17:19 +0200 Subject: [PATCH] ssh: fix spinloop in mux SendRequest drain on closed channel The drain loop added in 4e7a738 ("ssh: fix deadlock on unexpected global responses") receives from m.globalResponses without checking the comma-ok flag. Once mux.loop closes globalResponses on connection teardown, the loop spins forever on the zero value, pinning a CPU core for every concurrent SendRequest caller. Use the comma-ok idiom to detect the closed channel and break out. Change-Id: Icf0a7419fe5b206b7a4f70429fa9f25d0b5af731 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783681 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Neal Patel Auto-Submit: Nicola Murino Reviewed-by: Neal Patel Reviewed-by: Junyang Shao --- ssh/mux.go | 5 ++++- ssh/mux_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/ssh/mux.go b/ssh/mux.go index 3bc4afb..5775881 100644 --- a/ssh/mux.go +++ b/ssh/mux.go @@ -155,7 +155,10 @@ func (m *mux) SendRequest(name string, wantReply bool, payload []byte) (bool, [] drain: for { select { - case <-m.globalResponses: + case _, ok := <-m.globalResponses: + if !ok { + break drain + } default: break drain } diff --git a/ssh/mux_test.go b/ssh/mux_test.go index 48a8da6..a7dff34 100644 --- a/ssh/mux_test.go +++ b/ssh/mux_test.go @@ -1405,3 +1405,41 @@ closed: t.Fatal("SendRequest spinloop on closed ch.msg") } } + +func TestMuxSendRequestAfterCloseNoSpinloop(t *testing.T) { + a, b := muxPair() + defer a.Close() + + // Closing the underlying conn causes mux.loop to exit and close + // b.globalResponses. + if err := b.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + // Wait for globalResponses to actually be closed by loop(). + for { + select { + case _, ok := <-b.globalResponses: + if !ok { + goto closed + } + case <-time.After(2 * time.Second): + t.Fatal("globalResponses was not closed") + } + } +closed: + + done := make(chan error, 1) + go func() { + _, _, err := b.SendRequest("ping", true, nil) + done <- err + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("SendRequest after close: got nil error") + } + case <-time.After(2 * time.Second): + t.Fatal("SendRequest spinloop on closed globalResponses") + } +}