[systemd-commits] 2 commits - src/bus-proxyd

[David Herrmann]

 src/bus-proxyd/bus-proxyd.c |   36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

New commits:
commit b864535791844ce4f9437cebefaf3c37b3741b4a
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Thu Jan 8 21:06:14 2015 +0100

    bus-proxyd: fix EPERM on replies
    
    Imagine a kdbus peer sending a method-call without EXPECT_REPLY set
    through the proxy to a dbus1 peer. The proxy turns the missing
    EXPECT_REPLY flag into a dbus1 NO_REPLY_EXPECTED flag. However, if the
    receipient ignores that flag (valid dbus1 behavior) and sends a reply, the
    proxy will try to forward it to the original peer. This will fail with
    EPERM as the kernel didn't track the reply.
    
    We have two options now: Either we ignore EPERM for reply messages, or we
    track reply-windows in the proxy so we can properly ignore replies if
    EXPECT_REPLY wasn't set.
    
    This commit chose the first option: ignore EPERM for replies. The only
    down-side is that replies without matching method call will no longer be
    forwarded by the proxy. This works on dbus1, though.
    Nobody sane does this, so lets ignore it.

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 44e16fc..21cd4e2 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -1605,14 +1605,26 @@ int main(int argc, char *argv[]) {
                                 if (!processed) {
                                         k = sd_bus_send(b, m, NULL);
                                         if (k < 0) {
-                                                if (k == -ECONNRESET)
+                                                if (k == -ECONNRESET) {
                                                         r = 0;
-                                                else {
+                                                        goto finish;
+                                                } else if (k == -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. */
+                                                        r = 1;
+                                                } else {
                                                         r = k;
                                                         log_error_errno(r, "Failed to send message to client: %m");
+                                                        goto finish;
                                                 }
-
-                                                goto finish;
                                         } else
                                                 r = 1;
                                 }
@@ -1682,17 +1694,20 @@ int main(int argc, char *argv[]) {
 
                                                 k = sd_bus_send(a, m, NULL);
                                                 if (k < 0) {
-                                                        if (k == -EREMCHG)
+                                                        if (k == -EREMCHG) {
                                                                 /* The name database changed since the policy check, hence let's check again */
                                                                 continue;
-                                                        else if (k == -ECONNRESET)
+                                                        } else if (k == -ECONNRESET) {
                                                                 r = 0;
-                                                        else {
+                                                                goto finish;
+                                                        } else if (k == -EPERM && m->reply_cookie > 0) {
+                                                                /* see above why EPERM is ignored for replies */
+                                                                r = 1;
+                                                        } else {
                                                                 r = k;
                                                                 log_error_errno(r, "Failed to send message to bus: %m");
+                                                                goto finish;
                                                         }
-
-                                                        goto finish;
                                                 } else
                                                         r = 1;
 

commit 426bb5ddb8ff122d3e08b0480466718b68485e70
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Thu Jan 8 20:58:59 2015 +0100

    bus-proxyd: optimize replies if they're not requested
    
    If a caller does not request a reply, dont send it. This skips message
    creation and speeds up NO_REPLY_EXPECTED cases. Note that sd-bus still
    handles this case internally, but if we handle it in bus-proxyd, we can
    skip the whole message creation step.

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 549120b..44e16fc 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -405,6 +405,9 @@ static int synthetic_reply_return_strv(sd_bus_message *call, char **l) {
 
         assert(call);
 
+        if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED)
+                return 0;
+
         r = sd_bus_message_new_method_return(call, &m);
         if (r < 0)
                 return synthetic_reply_method_errno(call, r, NULL);