From e3e62d9601ec6fa737c081aead768f525f919802 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 27 May 2026 11:13:48 +0200 Subject: [PATCH] ssh: fix spinloop in channel SendRequest drain on closed channel The drain loop added in 3c7c869 ("ssh: fix deadlock on unexpected channel responses") receives from ch.msg without checking the comma-ok flag. Once ch.msg is closed by channel.close(), receives succeed immediately with the zero value, so the default arm is never taken and the loop spins forever, pinning a CPU core for every concurrent SendRequest caller on a torn-down channel. Use the comma-ok idiom to detect the closed channel and break out. Fixes golang/go#79658 Change-Id: Id80aa5d8429550c7a68e8759aed9ffba3aabb006 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783680 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Junyang Shao Reviewed-by: Neal Patel Auto-Submit: Nicola Murino Reviewed-by: Neal Patel --- ssh/channel.go | 5 ++++- ssh/mux_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/ssh/channel.go b/ssh/channel.go index 6737996..afc9aef 100644 --- a/ssh/channel.go +++ b/ssh/channel.go @@ -634,7 +634,10 @@ func (ch *channel) SendRequest(name string, wantReply bool, payload []byte) (boo drain: for { select { - case <-ch.msg: + case _, ok := <-ch.msg: + if !ok { + break drain + } default: break drain } diff --git a/ssh/mux_test.go b/ssh/mux_test.go index 65d22be..48a8da6 100644 --- a/ssh/mux_test.go +++ b/ssh/mux_test.go @@ -10,6 +10,7 @@ import ( "io" "sync" "testing" + "time" ) func muxPair() (*mux, *mux) { @@ -1366,3 +1367,41 @@ func TestChannelResponseAcceptedWhilePending(t *testing.T) { writer.Close() <-serverDone } + +func TestChannelSendRequestAfterCloseNoSpinloop(t *testing.T) { + writer, reader, mux := channelPair(t) + defer writer.Close() + defer mux.Close() + + // Close the channel, this closes reader.msg. + if err := reader.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + // Wait for reader.msg to actually be closed by the mux loop. + for { + select { + case _, ok := <-reader.msg: + if !ok { + goto closed + } + case <-time.After(2 * time.Second): + t.Fatal("reader.msg was not closed") + } + } +closed: + + done := make(chan error, 1) + go func() { + _, err := reader.SendRequest("ping", true, nil) + done <- err + }() + + select { + case err := <-done: + if !errors.Is(err, io.EOF) { + t.Fatalf("SendRequest after close: got %v, want io.EOF", err) + } + case <-time.After(2 * time.Second): + t.Fatal("SendRequest spinloop on closed ch.msg") + } +}