From f012617edfd887a29345888d65640a7ccd7c72ce Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Mon, 28 Nov 2022 14:45:34 -0800 Subject: [PATCH] CRI stream server: Fix goroutine leak in Exec In the CRI streaming server, a goroutine (`handleResizeEvents`) is launched to handle terminal resize events if a TTY is asked for with an exec; this is the sender of terminal resize events. Another goroutine is launched shortly after successful process startup to actually do something with these events, however the issue arises if the exec process fails to start for any reason that would have `process.Start` return non-nil. The receiver goroutine never gets launched so the sender is stuck blocked on a channel send infinitely. This could be used in a malicious manner by repeatedly launching execs with a command that doesn't exist in the image, as a single goroutine will get leaked on every invocation which will slowly grow containerd's memory usage. Signed-off-by: Danny Canter --- pkg/cri/streaming/remotecommand/httpstream.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/cri/streaming/remotecommand/httpstream.go b/pkg/cri/streaming/remotecommand/httpstream.go index 0417a1a9e..9177fa794 100644 --- a/pkg/cri/streaming/remotecommand/httpstream.go +++ b/pkg/cri/streaming/remotecommand/httpstream.go @@ -33,6 +33,7 @@ limitations under the License. package remotecommand import ( + gocontext "context" "encoding/json" "errors" "fmt" @@ -132,7 +133,7 @@ func createStreams(req *http.Request, w http.ResponseWriter, opts *Options, supp if ctx.resizeStream != nil { ctx.resizeChan = make(chan remotecommand.TerminalSize) - go handleResizeEvents(ctx.resizeStream, ctx.resizeChan) + go handleResizeEvents(req.Context(), ctx.resizeStream, ctx.resizeChan) } return ctx, true @@ -425,7 +426,7 @@ WaitForStreams: // supportsTerminalResizing returns false because v1ProtocolHandler doesn't support it. func (*v1ProtocolHandler) supportsTerminalResizing() bool { return false } -func handleResizeEvents(stream io.Reader, channel chan<- remotecommand.TerminalSize) { +func handleResizeEvents(ctx gocontext.Context, stream io.Reader, channel chan<- remotecommand.TerminalSize) { defer runtime.HandleCrash() defer close(channel) @@ -435,7 +436,15 @@ func handleResizeEvents(stream io.Reader, channel chan<- remotecommand.TerminalS if err := decoder.Decode(&size); err != nil { break } - channel <- size + + select { + case channel <- size: + case <-ctx.Done(): + // To avoid leaking this routine, exit if the http request finishes. This path + // would generally be hit if starting the process fails and nothing is started to + // ingest these resize events. + return + } } }