From 392cf1d05dbfa1395f6d99102e5ea41debb58fec Mon Sep 17 00:00:00 2001 From: Shawn Landden Date: Sat, 3 Feb 2018 10:16:33 -0800 Subject: [PATCH] sd-bus: cleanup ssh sessions (Closes: #8076) we still invoke ssh unnecessarily when there in incompatible or erreneous input The fallow-up to finish that would make the code a bit more verbose, as it would require repeating this bit: ``` r = bus_connect_transport(arg_transport, arg_host, false, &bus); if (r < 0) { log_error_errno(r, "Failed to create bus connection: %m"); goto finish; } sd_bus_set_allow_interactive_authorization(bus, arg_ask_password); ``` in every verb, after parsing. v2: add waitpid() to avoid a zombie process, switch to SIGTERM from SIGKILL v3: refactor, wait in bus_start_address() --- src/basic/process-util.c | 7 +++++++ src/basic/process-util.h | 1 + src/libsystemd/sd-bus/bus-internal.h | 1 + src/libsystemd/sd-bus/bus-socket.c | 4 ++-- src/libsystemd/sd-bus/sd-bus.c | 17 +++++++++++++++++ 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index e04bcc97827..7f8644ea9f4 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -806,6 +806,13 @@ void sigkill_waitp(pid_t *pid) { sigkill_wait(*pid); } +void sigterm_wait(pid_t pid) { + assert(pid > 1); + + if (kill_and_sigcont(pid, SIGTERM) > 0) + (void) wait_for_terminate(pid, NULL); +} + int kill_and_sigcont(pid_t pid, int sig) { int r; diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 9fabe4a5bed..93029e36e5f 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -76,6 +76,7 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout); void sigkill_wait(pid_t pid); void sigkill_waitp(pid_t *pid); +void sigterm_wait(pid_t pid); int kill_and_sigcont(pid_t pid, int sig); diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 1b55cdafed2..b305c41622b 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -296,6 +296,7 @@ struct sd_bus { unsigned n_memfd_cache; pid_t original_pid; + pid_t busexec_pid; sd_event_source *input_io_event_source; sd_event_source *output_io_event_source; diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 2fe86b61c4a..d2101028418 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -937,18 +937,18 @@ int bus_socket_connect(sd_bus *b) { int bus_socket_exec(sd_bus *b) { int s[2], r; - pid_t pid; assert(b); assert(b->input_fd < 0); assert(b->output_fd < 0); assert(b->exec_path); + assert(b->busexec_pid == 0); r = socketpair(AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0, s); if (r < 0) return -errno; - r = safe_fork_full("(sd-busexec)", s+1, 1, FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork_full("(sd-busexec)", s+1, 1, FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &b->busexec_pid); if (r < 0) { safe_close_pair(s); return r; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 7e7ebb27a72..24967508c8f 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -22,8 +22,10 @@ #include #include #include +#include #include #include +#include #include #include "sd-bus.h" @@ -1099,6 +1101,13 @@ static int bus_parse_next_address(sd_bus *b) { return 1; } +static void bus_kill_exec(sd_bus *bus) { + if (pid_is_valid(bus->busexec_pid) > 0) { + sigterm_wait(bus->busexec_pid); + bus->busexec_pid = 0; + } +} + static int bus_start_address(sd_bus *b) { int r; @@ -1108,6 +1117,8 @@ static int bus_start_address(sd_bus *b) { bus_close_io_fds(b); bus_close_inotify_fd(b); + bus_kill_exec(b); + /* If you provide multiple different bus-addresses, we * try all of them in order and use the first one that * succeeds. */ @@ -1506,6 +1517,9 @@ _public_ void sd_bus_close(sd_bus *bus) { if (bus_pid_changed(bus)) return; + /* Don't leave ssh hanging around */ + bus_kill_exec(bus); + bus_set_state(bus, BUS_CLOSED); sd_bus_detach_event(bus); @@ -1523,6 +1537,9 @@ _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) { if (!bus) return NULL; + /* Have to do this before flush() to prevent hang */ + bus_kill_exec(bus); + sd_bus_flush(bus); sd_bus_close(bus); -- 2.39.2