From c9cc06c9e3545716e7a5dba8d6bfaf2cf8ef0078 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 21 Dec 2018 12:43:39 +0100 Subject: ssh: Add port close test --- lib/ssh/test/ssh_basic_SUITE.erl | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index 365f25fabb..0131654dd0 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -37,6 +37,7 @@ cli/1, close/1, daemon_already_started/1, + daemon_error_closes_port/1, daemon_opt_fd/1, multi_daemon_opt_fd/1, double_close/1, @@ -108,6 +109,7 @@ all() -> {group, internal_error}, {group, rsa_host_key_is_actualy_ecdsa}, daemon_already_started, + daemon_error_closes_port, double_close, daemon_opt_fd, multi_daemon_opt_fd, @@ -796,6 +798,24 @@ daemon_already_started(Config) when is_list(Config) -> fun ssh_test_lib:failfun/2}]), ssh:stop_daemon(Pid). +%%-------------------------------------------------------------------- +%%% Test that a failed daemon start does not leave the port open +daemon_error_closes_port(Config) -> + GoodSystemDir = proplists:get_value(data_dir, Config), + Port = ssh_test_lib:inet_port(), + {error,_} = ssh_test_lib:daemon(Port, []), % No system dir + case ssh_test_lib:daemon(Port, [{system_dir, GoodSystemDir}]) of + {error,eaddrinuse} -> + {fail, "Port leakage"}; + {error,Error} -> + ct:log("Strange error: ~p",[Error]), + {fail, "Strange error"}; + {Pid, _Host, Port} -> + %% Ok + ssh:stop_daemon(Pid) + end. + + %%-------------------------------------------------------------------- %%% check that known_hosts is updated correctly known_hosts(Config) when is_list(Config) -> -- cgit v1.2.3 From 9b8575437a3c84eab77453279d8844c52485216c Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 19 Dec 2018 18:07:20 +0100 Subject: ssh: Fix port leakage for daemons failing at start --- lib/ssh/src/ssh.erl | 60 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 25d537c624..5f5ccee4f8 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -231,25 +231,38 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535, try {Host1, UserOptions} = handle_daemon_args(Host0, UserOptions0), #{} = Options0 = ssh_options:handle_options(server, UserOptions), - - {{Host,Port}, ListenSocket} = - open_listen_socket(Host1, Port0, Options0), - - %% Now Host,Port is what to use for the supervisor to register its name, - %% and ListenSocket is for listening on connections. But it is still owned - %% by self()... - - finalize_start(Host, Port, ?GET_OPT(profile, Options0), - ?PUT_INTERNAL_OPT({lsocket,{ListenSocket,self()}}, Options0), - fun(Opts, Result) -> - {_, Callback, _} = ?GET_OPT(transport, Opts), - receive - {request_control, ListenSocket, ReqPid} -> - ok = Callback:controlling_process(ListenSocket, ReqPid), - ReqPid ! {its_yours,ListenSocket}, - Result - end - end) + {open_listen_socket(Host1, Port0, Options0), Options0} + of + {{{Host,Port}, ListenSocket}, Options1} -> + try + %% Now Host,Port is what to use for the supervisor to register its name, + %% and ListenSocket is for listening on connections. But it is still owned + %% by self()... + finalize_start(Host, Port, ?GET_OPT(profile, Options1), + ?PUT_INTERNAL_OPT({lsocket,{ListenSocket,self()}}, Options1), + fun(Opts, Result) -> + {_, Callback, _} = ?GET_OPT(transport, Opts), + receive + {request_control, ListenSocket, ReqPid} -> + ok = Callback:controlling_process(ListenSocket, ReqPid), + ReqPid ! {its_yours,ListenSocket}, + Result + end + end) + of + {error,Err} -> + close_listen_socket(ListenSocket, Options1), + {error,Err}; + OK -> + OK + catch + error:Error -> + close_listen_socket(ListenSocket, Options1), + error(Error); + exit:Exit -> + close_listen_socket(ListenSocket, Options1), + exit(Exit) + end catch throw:bad_fd -> {error,bad_fd}; @@ -456,6 +469,15 @@ open_listen_socket(_Host0, Port0, Options0) -> {ok,{LHost,LPort}} = inet:sockname(LSock), {{LHost,LPort}, LSock}. +%%%---------------------------------------------------------------- +close_listen_socket(ListenSocket, Options) -> + try + {_, Callback, _} = ?GET_OPT(transport, Options), + Callback:close(ListenSocket) + catch + _C:_E -> ok + end. + %%%---------------------------------------------------------------- finalize_start(Host, Port, Profile, Options0, F) -> try -- cgit v1.2.3