bus-proxy: whenever we cannot forward a message, report this back to caller, but...

Errors like EPERM from the kernel should certainly not be reason to
exit. Let's try to be defensive here, and try to continue on most send
errors, but possibly tell the sender about it.

diff --git a/src/bus-proxyd/proxy.c b/src/bus-proxyd/proxy.c
index e3042d827d747f1ad1ebb66c07e08da3319f17a4..a07c4036dfdb944eeedae5db5d0c6f11a1c3879c 100644 (file)
--- a/src/bus-proxyd/proxy.c
+++ b/src/bus-proxyd/proxy.c
@@ -704,23 +704,29 @@ static int proxy_process_destination_to_local(Proxy *p) {
 
         r = sd_bus_send(p->local_bus, m, NULL);
         if (r < 0) {
-                if (r == -EPERM && m->reply_cookie > 0) {
-                        /* If the peer tries to send a reply and it is rejected with EPERM
-                         * by the kernel, we ignore the error. This catches cases where the
-                         * original method-call didn't had EXPECT_REPLY set, but the proxy-peer
-                         * still sends a reply. This is allowed in dbus1, but not in kdbus. We
-                         * don't want to track reply-windows in the proxy, so we simply ignore
-                         * EPERM for all replies. The only downside is, that callers are no
-                         * longer notified if their replies are dropped. However, this is
-                         * equivalent to the caller's timeout to expire, so this should be
-                         * acceptable. Nobody sane sends replies without a matching method-call,
-                         * so nobody should care. */
-                        return 1;
-                } else {
-                        if (r != -ECONNRESET)
-                                log_error_errno(r, "Failed to send message to client: %m");
+                if (r == -ECONNRESET)
                         return r;
-                }
+
+                /* If the peer tries to send a reply and it is
+                 * rejected with EPERM by the kernel, we ignore the
+                 * error. This catches cases where the original
+                 * method-call didn't had EXPECT_REPLY set, but the
+                 * proxy-peer still sends a reply. This is allowed in
+                 * dbus1, but not in kdbus. We don't want to track
+                 * reply-windows in the proxy, so we simply ignore
+                 * EPERM for all replies. The only downside is, that
+                 * callers are no longer notified if their replies are
+                 * dropped. However, this is equivalent to the
+                 * caller's timeout to expire, so this should be
+                 * acceptable. Nobody sane sends replies without a
+                 * matching method-call, so nobody should care. */
+                if (r == -EPERM && m->reply_cookie > 0)
+                        return 1;
+
+                /* Return the error to the client, if we can */
+                synthetic_reply_method_errnof(m, r, "Failed to forward message we got from destination: %m");
+                log_error_errno(r, "Failed to send message to client, ignoring: %m");
+                return 1;
         }
 
         return 1;
@@ -771,17 +777,20 @@ static int proxy_process_local_to_destination(Proxy *p) {
 
                 r = sd_bus_send(p->destination_bus, m, NULL);
                 if (r < 0) {
-                        if (r == -EREMCHG) {
-                                /* The name database changed since the policy check, hence let's check again */
+                        if (r == -ECONNRESET)
+                                return r;
+
+                        /* The name database changed since the policy check, hence let's check again */
+                        if (r == -EREMCHG)
                                 continue;
-                        } else if (r == -EPERM && m->reply_cookie > 0) {
-                                /* see above why EPERM is ignored for replies */
+
+                        /* see above why EPERM is ignored for replies */
+                        if (r == -EPERM && m->reply_cookie > 0)
                                 return 1;
-                        } else {
-                                if (r != -ECONNRESET)
-                                        log_error_errno(r, "Failed to send message to bus: %m");
-                                return r;
-                        }
+
+                        synthetic_reply_method_errnof(m, r, "Failed to forward message we got from local: %m");
+                        log_error_errno(r, "Failed to send message to bus: %m");
+                        return 1;
                 }
 
                 break;

diff --git a/src/bus-proxyd/synthesize.c b/src/bus-proxyd/synthesize.c
index 36ffe2930e94a722941c8bfbab210ebb894049b7..e1b0fd35354b3fccb94fac59d1e846134069088c 100644 (file)
--- a/src/bus-proxyd/synthesize.c
+++ b/src/bus-proxyd/synthesize.c
@@ -83,7 +83,6 @@ int synthetic_reply_method_errorf(sd_bus_message *call, const char *name, const
 }
 
 int synthetic_reply_method_errno(sd_bus_message *call, int error, const sd_bus_error *p) {
-
         _cleanup_bus_error_free_ sd_bus_error berror = SD_BUS_ERROR_NULL;
 
         assert(call);
@@ -99,6 +98,22 @@ int synthetic_reply_method_errno(sd_bus_message *call, int error, const sd_bus_e
         return synthetic_reply_method_error(call, &berror);
 }
 
+int synthetic_reply_method_errnof(sd_bus_message *call, int error, const char *format, ...) {
+        _cleanup_bus_error_free_ sd_bus_error berror = SD_BUS_ERROR_NULL;
+        va_list ap;
+
+        assert(call);
+
+        if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED)
+                return 0;
+
+        va_start(ap, format);
+        sd_bus_error_set_errnofv(&berror, error, format, ap);
+        va_end(ap);
+
+        return synthetic_reply_method_error(call, &berror);
+}
+
 int synthetic_reply_method_return(sd_bus_message *call, const char *types, ...) {
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
         int r;

diff --git a/src/bus-proxyd/synthesize.h b/src/bus-proxyd/synthesize.h
index e93e0d34daaf82e2f62741ba31c74fcdeb8eda82..a55f171cb2a327cc11e8aa34be231dfe7d69eb41 100644 (file)
--- a/src/bus-proxyd/synthesize.h
+++ b/src/bus-proxyd/synthesize.h
@@ -23,10 +23,12 @@
 
 #include "sd-bus.h"
 
-int synthetic_reply_method_errorf(sd_bus_message *call, const char *name, const char *format, ...);
 int synthetic_reply_method_return(sd_bus_message *call, const char *types, ...);
 int synthetic_reply_method_return_strv(sd_bus_message *call, char **l);
 
 int synthetic_reply_method_error(sd_bus_message *call, const sd_bus_error *e);
+int synthetic_reply_method_errorf(sd_bus_message *call, const char *name, const char *format, ...) _sd_printf_(3, 4);
 int synthetic_reply_method_errno(sd_bus_message *call, int error, const sd_bus_error *p);
+int synthetic_reply_method_errnof(sd_bus_message *call, int error, const char *format, ...) _sd_printf_(3, 4);
+
 int synthesize_name_acquired(sd_bus *a, sd_bus *b, sd_bus_message *m);