Merge pull request #1595 from poettering/proxy-fixes

bus proxy fixes, and more

3 files changed, 41 insertions, 22 deletions

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 2bc265d9b4..64d1c5231f 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -85,11 +85,11 @@ static void *run_client(void *userdata) {
         int r;
 
         r = proxy_new(&p, c->fd, c->fd, arg_address);
+        c->fd = -1;
+
         if (r < 0)
                 goto exit;
 
-        c->fd = -1;
-
         /* set comm to "p$PIDu$UID" and suffix with '*' if truncated */
         r = snprintf(comm, sizeof(comm), "p" PID_FMT "u" UID_FMT, p->local_creds.pid, p->local_creds.uid);
         if (r >= (ssize_t)sizeof(comm))
@@ -116,13 +116,12 @@ static int loop_clients(int accept_fd, uid_t bus_uid) {
         int r;
 
         r = pthread_attr_init(&attr);
-        if (r < 0) {
-                return log_error_errno(errno, "Cannot initialize pthread attributes: %m");
-        }
+        if (r != 0)
+                return log_error_errno(r, "Cannot initialize pthread attributes: %m");
 
         r = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-        if (r < 0) {
-                r = log_error_errno(errno, "Cannot mark pthread attributes as detached: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot mark pthread attributes as detached: %m");
                 goto finish;
         }
 
@@ -156,8 +155,8 @@ static int loop_clients(int accept_fd, uid_t bus_uid) {
                 c->bus_uid = bus_uid;
 
                 r = pthread_create(&tid, &attr, run_client, c);
-                if (r < 0) {
-                        log_error("Cannot spawn thread: %m");
+                if (r != 0) {
+                        log_warning_errno(r, "Cannot spawn thread, ignoring: %m");
                         client_context_free(c);
                         continue;
                 }
diff --git a/src/bus-proxyd/bus-xml-policy.c b/src/bus-proxyd/bus-xml-policy.c
index 9a3b451c56..91717653c2 100644
--- a/src/bus-proxyd/bus-xml-policy.c
+++ b/src/bus-proxyd/bus-xml-policy.c
@@ -1186,14 +1186,14 @@ int shared_policy_new(SharedPolicy **out) {
                 return log_oom();
 
         r = pthread_mutex_init(&sp->lock, NULL);
-        if (r < 0) {
-                log_error_errno(r, "Cannot initialize shared policy mutex: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot initialize shared policy mutex: %m");
                 goto exit_free;
         }
 
         r = pthread_rwlock_init(&sp->rwlock, NULL);
-        if (r < 0) {
-                log_error_errno(r, "Cannot initialize shared policy rwlock: %m");
+        if (r != 0) {
+                r = log_error_errno(r, "Cannot initialize shared policy rwlock: %m");
                 goto exit_mutex;
         }
 
diff --git a/src/bus-proxyd/proxy.c b/src/bus-proxyd/proxy.c
index 88800f5e7f..bc8516f5c6 100644
--- a/src/bus-proxyd/proxy.c
+++ b/src/bus-proxyd/proxy.c
@@ -100,18 +100,24 @@ static int proxy_create_destination(Proxy *p, const char *destination, const cha
         return 0;
 }
 
-static int proxy_create_local(Proxy *p, int in_fd, int out_fd, bool negotiate_fds) {
-        _cleanup_bus_flush_close_unref_ sd_bus *b = NULL;
+static int proxy_create_local(Proxy *p, bool negotiate_fds) {
         sd_id128_t server_id;
+        sd_bus *b;
         int r;
 
         r = sd_bus_new(&b);
         if (r < 0)
                 return log_error_errno(r, "Failed to allocate bus: %m");
 
-        r = sd_bus_set_fd(b, in_fd, out_fd);
-        if (r < 0)
+        r = sd_bus_set_fd(b, p->local_in, p->local_out);
+        if (r < 0) {
+                sd_bus_unref(b);
                 return log_error_errno(r, "Failed to set fds: %m");
+        }
+
+        /* The fds are now owned by the bus, and we indicate that by
+         * storing the bus object in the proxy object. */
+        p->local_bus = b;
 
         r = sd_bus_get_bus_id(p->destination_bus, &server_id);
         if (r < 0)
@@ -139,8 +145,6 @@ static int proxy_create_local(Proxy *p, int in_fd, int out_fd, bool negotiate_fd
         if (r < 0)
                 return log_error_errno(r, "Failed to start bus client: %m");
 
-        p->local_bus = b;
-        b = NULL;
         return 0;
 }
 
@@ -224,9 +228,17 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
         bool is_unix;
         int r;
 
+        /* This takes possession/destroys the file descriptors passed
+         * in even on failure. The caller should hence forget about
+         * the fds in all cases after calling this function and not
+         * close them. */
+
         p = new0(Proxy, 1);
-        if (!p)
+        if (!p) {
+                safe_close(in_fd);
+                safe_close(out_fd);
                 return log_oom();
+        }
 
         p->local_in = in_fd;
         p->local_out = out_fd;
@@ -247,7 +259,7 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
         if (r < 0)
                 return r;
 
-        r = proxy_create_local(p, in_fd, out_fd, is_unix);
+        r = proxy_create_local(p, is_unix);
         if (r < 0)
                 return r;
 
@@ -257,6 +269,7 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
 
         *out = p;
         p = NULL;
+
         return 0;
 }
 
@@ -273,7 +286,14 @@ Proxy *proxy_free(Proxy *p) {
                 free(activation);
         }
 
-        sd_bus_flush_close_unref(p->local_bus);
+        if (p->local_bus)
+                sd_bus_flush_close_unref(p->local_bus);
+        else {
+                safe_close(p->local_in);
+                if (p->local_out != p->local_in)
+                        safe_close(p->local_out);
+        }
+
         sd_bus_flush_close_unref(p->destination_bus);
         set_free_free(p->owned_names);
         free(p);