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;
}