From 53b994b06e58dd42785e9257c97d2545a563bf59 Mon Sep 17 00:00:00 2001 From: Jakub Witczak Date: Fri, 5 Jun 2026 18:37:41 +0200 Subject: [PATCH] ssh: Fix keepalive response stealing channel open requests A keepalive response (ssh_msg_request_success) could consume a pending channel open entry from the shared requests queue, causing the channel setup to fail silently with a timeout. Replace positional head-matching of the requests list with take_global_request/1 that selects entries by reference key, skipping integer-keyed channel entries. Register keepalive probes in the queue so their responses are properly consumed. --- lib/ssh/src/ssh_connection.erl | 61 ++++++++++++++++---------- lib/ssh/src/ssh_connection_handler.erl | 4 +- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/lib/ssh/src/ssh_connection.erl b/lib/ssh/src/ssh_connection.erl index 40fdf03dc27d..0628154ad56a 100644 --- a/lib/ssh/src/ssh_connection.erl +++ b/lib/ssh/src/ssh_connection.erl @@ -1291,36 +1291,49 @@ handle_msg(#ssh_msg_global_request{name = _Type, end; handle_msg(#ssh_msg_request_failure{}, - #connection{requests = [{_, From} | Rest]} = Connection, _, _SSH) -> - {[{channel_request_reply, From, {failure, <<>>}}], - Connection#connection{requests = Rest}}; - -handle_msg(#ssh_msg_request_failure{}, - #connection{requests = [{_, From,_} | Rest]} = Connection, _, _SSH) -> - {[{channel_request_reply, From, {failure, <<>>}}], - Connection#connection{requests = Rest}}; + #connection{requests = Requests} = Connection, _, _SSH) -> + handle_global_response({failure, <<>>}, Requests, Connection); handle_msg(#ssh_msg_request_success{data = Data}, - #connection{requests = [{_, From} | Rest]} = Connection, _, _SSH) -> - {[{channel_request_reply, From, {success, Data}}], - Connection#connection{requests = Rest}}; + #connection{requests = Requests} = Connection, _, _SSH) -> + handle_global_response({success, Data}, Requests, Connection). -handle_msg(#ssh_msg_request_success{data = Data}, - #connection{requests = [{_, From, Fun} | Rest]} = Connection0, _, _SSH) -> - Connection = Fun({success,Data}, Connection0), - {[{channel_request_reply, From, {success, Data}}], - Connection#connection{requests = Rest}}; - -%% alive responses -handle_msg(#ssh_msg_request_success{}, - #connection{requests = []} = Connection, _, _SSH) -> - {[], Connection}; -handle_msg(#ssh_msg_request_failure{}, - #connection{requests = []} = Connection, _, _SSH) -> - {[], Connection}. + +%%%---------------------------------------------------------------- +%%% Handle a global request response (success or failure). +%%% Finds the first global request entry and delivers the reply. +%%% +handle_global_response(Reply, Requests, Connection0) -> + case take_global_request(Requests) of + {{_, From}, Rest} -> + {[{channel_request_reply, From, Reply}], + Connection0#connection{requests = Rest}}; + {{_, From, Fun}, Rest} -> + Connection = Fun(Reply, Connection0), + {[{channel_request_reply, From, Reply}], + Connection#connection{requests = Rest}}; + false -> + {[], Connection0} + end. +%%%---------------------------------------------------------------- +%%% Find and remove the first global request entry (keyed by reference) +%%% from the requests list, skipping channel entries (keyed by integer). +%%% Returns {Entry, RemainingList} or false. +%%% +take_global_request([]) -> + false; +take_global_request([{Ref, _} = E | Rest]) when is_reference(Ref) -> + {E, Rest}; +take_global_request([{Ref, _, _} = E | Rest]) when is_reference(Ref) -> + {E, Rest}; +take_global_request([H | T]) -> + case take_global_request(T) of + {E, Rest} -> {E, [H | Rest]}; + false -> false + end. %%%---------------------------------------------------------------- %%% Returns pending responses to be delivered to the peer when a diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 304fa358d10f..f2b5ba4e43a4 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -2105,6 +2105,8 @@ get_repl({channel_data,undefined,_Data}, Acc) -> get_repl({channel_data,Pid,Data}, Acc) -> Pid ! {ssh_cm, self(), Data}, Acc; +get_repl({channel_request_reply,undefined,_Data}, Acc) -> + Acc; get_repl({channel_request_reply,From,Data}, {CallRepls,S}) -> {[{reply,From,Data}|CallRepls], S}; get_repl({flow_control,Cache,Channel,From,Msg}, {CallRepls,S}) -> @@ -2226,7 +2228,7 @@ triggered_alive(StateName, D0 = #data{}, {stop, Shutdown, D}; _ -> D = send_msg({ssh_msg_global_request,"keepalive@erlang.org", true, <<>>}, - D0), + add_request(fun(_,Conn) -> Conn end, make_ref(), undefined, D0)), Ssh = D#data.ssh_params, Now = erlang:monotonic_time(milli_seconds), Ssh1 = Ssh#ssh{alive_probes_sent = SentProbes + 1,