From 781a055c17400e953bb7929434fe7a2e6517d5e8 Mon Sep 17 00:00:00 2001

From: Lennart Poettering <lennart@poettering.net>

Date: Thu, 17 Jan 2019 18:31:59 +0100

Subject: [PATCH] sd-bus: use "queue" message references for managing r/w

 message queues in connection objects

Let's make use of the new concept the previous commit added.

See: #4846

(cherry picked from commit c1757a70eac0382c4837a3833d683919f6a48ed7)

Related: CVE-2020-1712

---

 src/libsystemd/sd-bus/bus-socket.c |  6 ++-

 src/libsystemd/sd-bus/sd-bus.c     | 60 ++++++++++++++----------------

 2 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c

index 17cfa8e1fd..4a72795d2b 100644

--- a/src/libsystemd/sd-bus/bus-socket.c

+++ b/src/libsystemd/sd-bus/bus-socket.c

@@ -1116,8 +1116,10 @@ static int bus_socket_make_message(sd_bus *bus, size_t size) {

         bus->fds = NULL;

         bus->n_fds = 0;

-        if (t)

-                bus->rqueue[bus->rqueue_size++] = t;

+        if (t) {

+                bus->rqueue[bus->rqueue_size++] = bus_message_ref_queued(t, bus);

+                sd_bus_message_unref(t);

+        }

         return 1;

 }

diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c

index 68ad6cbe89..a3509f7e89 100644

--- a/src/libsystemd/sd-bus/sd-bus.c

+++ b/src/libsystemd/sd-bus/sd-bus.c

@@ -148,13 +148,13 @@ static void bus_reset_queues(sd_bus *b) {

         assert(b);

         while (b->rqueue_size > 0)

-                sd_bus_message_unref(b->rqueue[--b->rqueue_size]);

+                bus_message_unref_queued(b->rqueue[--b->rqueue_size], b);

         b->rqueue = mfree(b->rqueue);

         b->rqueue_allocated = 0;

         while (b->wqueue_size > 0)

-                sd_bus_message_unref(b->wqueue[--b->wqueue_size]);

+                bus_message_unref_queued(b->wqueue[--b->wqueue_size], b);

         b->wqueue = mfree(b->wqueue);

         b->wqueue_allocated = 0;

@@ -493,7 +493,7 @@ static int synthesize_connected_signal(sd_bus *bus) {

         /* Insert at the very front */

         memmove(bus->rqueue + 1, bus->rqueue, sizeof(sd_bus_message*) * bus->rqueue_size);

-        bus->rqueue[0] = TAKE_PTR(m);

+        bus->rqueue[0] = bus_message_ref_queued(m, bus);

         bus->rqueue_size++;

         return 0;

@@ -1760,7 +1760,7 @@ static int dispatch_wqueue(sd_bus *bus) {

                          * anyway. */

                         bus->wqueue_size--;

-                        sd_bus_message_unref(bus->wqueue[0]);

+                        bus_message_unref_queued(bus->wqueue[0], bus);

                         memmove(bus->wqueue, bus->wqueue + 1, sizeof(sd_bus_message*) * bus->wqueue_size);

                         bus->windex = 0;

@@ -1789,6 +1789,15 @@ int bus_rqueue_make_room(sd_bus *bus) {

         return 0;

 }

+static void rqueue_drop_one(sd_bus *bus, size_t i) {

+        assert(bus);

+        assert(i < bus->rqueue_size);

+

+        bus_message_unref_queued(bus->rqueue[i], bus);

+        memmove(bus->rqueue + i, bus->rqueue + i + 1, sizeof(sd_bus_message*) * (bus->rqueue_size - i - 1));

+        bus->rqueue_size--;

+}

+

 static int dispatch_rqueue(sd_bus *bus, bool hint_priority, int64_t priority, sd_bus_message **m) {

         int r, ret = 0;

@@ -1803,10 +1812,8 @@ static int dispatch_rqueue(sd_bus *bus, bool hint_priority, int64_t priority, sd

         for (;;) {

                 if (bus->rqueue_size > 0) {

                         /* Dispatch a queued message */

-

-                        *m = bus->rqueue[0];

-                        bus->rqueue_size--;

-                        memmove(bus->rqueue, bus->rqueue + 1, sizeof(sd_bus_message*) * bus->rqueue_size);

+                        *m = sd_bus_message_ref(bus->rqueue[0]);

+                        rqueue_drop_one(bus, 0);

                         return 1;

                 }

@@ -1884,7 +1891,7 @@ _public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie) {

                          * of the wqueue array is always allocated so

                          * that we always can remember how much was

                          * written. */

-                        bus->wqueue[0] = sd_bus_message_ref(m);

+                        bus->wqueue[0] = bus_message_ref_queued(m, bus);

                         bus->wqueue_size = 1;

                         bus->windex = idx;

                 }

@@ -1898,7 +1905,7 @@ _public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie) {

                 if (!GREEDY_REALLOC(bus->wqueue, bus->wqueue_allocated, bus->wqueue_size + 1))

                         return -ENOMEM;

-                bus->wqueue[bus->wqueue_size++] = sd_bus_message_ref(m);

+                bus->wqueue[bus->wqueue_size++] = bus_message_ref_queued(m, bus);

         }

 finish:

@@ -2124,37 +2131,30 @@ _public_ int sd_bus_call(

                 usec_t left;

                 while (i < bus->rqueue_size) {

-                        sd_bus_message *incoming = NULL;

+                        _cleanup_(sd_bus_message_unrefp) sd_bus_message *incoming = NULL;

-                        incoming = bus->rqueue[i];

+                        incoming = sd_bus_message_ref(bus->rqueue[i]);

                         if (incoming->reply_cookie == cookie) {

                                 /* Found a match! */

-                                memmove(bus->rqueue + i, bus->rqueue + i + 1, sizeof(sd_bus_message*) * (bus->rqueue_size - i - 1));

-                                bus->rqueue_size--;

+                                rqueue_drop_one(bus, i);

                                 log_debug_bus_message(incoming);

                                 if (incoming->header->type == SD_BUS_MESSAGE_METHOD_RETURN) {

                                         if (incoming->n_fds <= 0 || bus->accept_fd) {

                                                 if (reply)

-                                                        *reply = incoming;

-                                                else

-                                                        sd_bus_message_unref(incoming);

+                                                        *reply = TAKE_PTR(incoming);

                                                 return 1;

                                         }

-                                        r = sd_bus_error_setf(error, SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Reply message contained file descriptors which I couldn't accept. Sorry.");

-                                        sd_bus_message_unref(incoming);

-                                        return r;

+                                        return sd_bus_error_setf(error, SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Reply message contained file descriptors which I couldn't accept. Sorry.");

-                                } else if (incoming->header->type == SD_BUS_MESSAGE_METHOD_ERROR) {

-                                        r = sd_bus_error_copy(error, &incoming->error);

-                                        sd_bus_message_unref(incoming);

-                                        return r;

-                                } else {

+                                } else if (incoming->header->type == SD_BUS_MESSAGE_METHOD_ERROR)

+                                        return sd_bus_error_copy(error, &incoming->error);

+                                else {

                                         r = -EIO;

                                         goto fail;

                                 }

@@ -2164,15 +2164,11 @@ _public_ int sd_bus_call(

                                    incoming->sender &&

                                    streq(bus->unique_name, incoming->sender)) {

-                                memmove(bus->rqueue + i, bus->rqueue + i + 1, sizeof(sd_bus_message*) * (bus->rqueue_size - i - 1));

-                                bus->rqueue_size--;

+                                rqueue_drop_one(bus, i);

-                                /* Our own message? Somebody is trying

-                                 * to send its own client a message,

-                                 * let's not dead-lock, let's fail

-                                 * immediately. */

+                                /* Our own message? Somebody is trying to send its own client a message,

+                                 * let's not dead-lock, let's fail immediately. */

-                                sd_bus_message_unref(incoming);

                                 r = -ELOOP;

                                 goto fail;

                         }