chiark / gitweb /
sd-bus: cleanup ssh sessions (Closes: #8076)
authorShawn Landden <slandden@gmail.com>
Sat, 3 Feb 2018 18:16:33 +0000 (10:16 -0800)
committerSven Eden <yamakuzure@gmx.net>
Wed, 30 May 2018 05:54:00 +0000 (07:54 +0200)
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()
(cherry picked from commit 392cf1d05dbfa1395f6d99102e5ea41debb58fec)

src/basic/process-util.c
src/basic/process-util.h
src/libelogind/sd-bus/bus-internal.h
src/libelogind/sd-bus/bus-socket.c
src/libelogind/sd-bus/sd-bus.c

index ba8b3d80f64bc8378945368f7d35ef5c49d51d58..ccf828389c2f07f6908db8778a91db0758edbc29 100644 (file)
@@ -813,6 +813,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;
 
index 7eae183d3e5297b2d40b9a3d6cc04c5abbceb928..82b1de03c8c8a54b853d8002e1f4f67c4b9f0151 100644 (file)
@@ -79,6 +79,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);
 
@@ -126,8 +127,13 @@ int sched_policy_to_string_alloc(int i, char **s);
 int sched_policy_from_string(const char *s);
 #endif // 0
 
-#define PTR_TO_PID(p) ((pid_t) ((uintptr_t) p))
-#define PID_TO_PTR(p) ((void*) ((uintptr_t) p))
+static inline pid_t PTR_TO_PID(const void *p) {
+        return (pid_t) ((uintptr_t) p);
+}
+
+static inline void* PID_TO_PTR(pid_t pid) {
+        return (void*) ((uintptr_t) pid);
+}
 
 void valgrind_summary_hack(void);
 
index 0aba536341f7f98eac45de9ded6c765d60cc33ba..14d2e32952450d8626c5ec9f3faacaed7ad5a4ef 100644 (file)
@@ -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;
index bd0895f275ac36d7e51dc8f9e95c613810299105..348f715809a6075dfecf3dade7d8d7aade192d0e 100644 (file)
@@ -621,13 +621,10 @@ static void bus_get_peercred(sd_bus *b) {
 
         /* Get the list of auxiliary groups of the peer */
         r = getpeergroups(b->input_fd, &b->groups);
-        if (r < 0) {
-                if (!IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT))
-                        log_debug_errno(r, "Failed to determine peer groups list: %m");
-
-                b->n_groups = (size_t) -1;
-        } else
+        if (r >= 0)
                 b->n_groups = (size_t) r;
+        else if (!IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT))
+                log_debug_errno(r, "Failed to determine peer's group list: %m");
 }
 
 static int bus_socket_start_auth_client(sd_bus *b) {
@@ -940,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;
index f11cdce850b8d73cd7c8751b826cd49094900c7e..072b3df0c1e44fc508e45ab425ab6adf8cd18fbd 100644 (file)
 #include <netdb.h>
 #include <poll.h>
 #include <pthread.h>
+//#include <signal.h>
 #include <stdlib.h>
 #include <sys/mman.h>
+//#include <sys/wait.h>
 #include <unistd.h>
 
 #include "sd-bus.h"
@@ -1107,6 +1109,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;
 
@@ -1116,6 +1125,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. */
@@ -1529,6 +1540,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);
@@ -1546,6 +1560,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);