sd-bus: fix freeing kdbus messages sandbox/adrians/upgrade-to-242
authorAdrian Szyndela <adrian.s@samsung.com>
Thu, 26 Mar 2020 07:13:50 +0000 (08:13 +0100)
committerAdrian Szyndela <adrian.s@samsung.com>
Fri, 27 Mar 2020 08:14:14 +0000 (09:14 +0100)
Version 242 introduced two kinds of references to messages: normal and queued.
This commit fixes this functionality for kdbus:
- (normal_)unreference messages when they are (queue_)referenced;
- fix freeing function to allow release of kdbus memory.

src/libsystemd/sd-bus/bus-control-kernel.c
src/libsystemd/sd-bus/bus-kernel.c
src/libsystemd/sd-bus/bus-message.c

index 95d05bd..76b9852 100644 (file)
@@ -225,7 +225,6 @@ static int enqueue_int_kernel_reply(
         if (ret < 0)
             return ret;
 
-        m = NULL;
         return 1;
 }
 
@@ -1256,10 +1255,7 @@ int bus_add_match_internal_kernel_async(
     if (ret < 0)
         return ret;
 
-    ret = enqueue_kernel_reply(bus, ret_slot, m, callback, userdata);
-    m = NULL;
-
-    return ret;
+    return enqueue_kernel_reply(bus, ret_slot, m, callback, userdata);
 }
 
 int bus_remove_match_internal_kernel(
index 4d2faf5..fd4eedd 100644 (file)
@@ -856,6 +856,7 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k) {
         fds = NULL;
 
         bus->rqueue[bus->rqueue_size++] = bus_message_ref_queued(m, bus);
+        sd_bus_message_unref(m);
 
         return 1;
 
@@ -1128,7 +1129,7 @@ int bus_kernel_write_message(sd_bus *bus, sd_bus_message *m, bool hint_sync_call
         r = ioctl(bus->output_fd, KDBUS_CMD_SEND, &cmd);
         if (r < 0) {
                 _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-                sd_bus_message *reply;
+                _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
 
                 if (errno == EAGAIN || errno == EINTR)
                         return 0;
@@ -1232,7 +1233,6 @@ static int push_name_owner_changed(
                 return r;
 
         bus->rqueue[bus->rqueue_size++] = bus_message_ref_queued(m, bus);
-        m = NULL;
 
         return 1;
 }
@@ -1317,7 +1317,6 @@ static int translate_reply(
                 return r;
 
         bus->rqueue[bus->rqueue_size++] = bus_message_ref_queued(m, bus);
-        m = NULL;
 
         return 1;
 }
index 4e841a7..4640054 100644 (file)
@@ -131,8 +131,11 @@ static sd_bus_message* message_free(sd_bus_message *m) {
 
         message_reset_parts(m);
 
-        if (m->release_kdbus)
+        if (m->release_kdbus) {
                 bus_kernel_cmd_free(m->bus, (uint8_t *) m->kdbus - (uint8_t *) m->bus->kdbus_buffer);
+                sd_bus_unref(m->bus);
+                m->bus = NULL;
+        }
 
         if (m->free_kdbus)
                 free(m->kdbus);
@@ -971,20 +974,25 @@ _public_ sd_bus_message* sd_bus_message_unref(sd_bus_message *m) {
 
         assert(m->n_ref > 0);
 
-        sd_bus_unref(m->bus); /* Each regular ref is also a ref on the bus connection. Let's hence drop it
-                               * here. Note we have to do this before decrementing our own n_ref here, since
-                               * otherwise, if this message is currently queued sd_bus_unref() might call
-                               * bus_message_unref_queued() for this which might then destroy the message
-                               * while we are still processing it. */
+        if (!m->release_kdbus)
+                sd_bus_unref(m->bus); /* Each regular ref is also a ref on the bus connection. Let's hence drop it
+                                       * here. Note we have to do this before decrementing our own n_ref here, since
+                                       * otherwise, if this message is currently queued sd_bus_unref() might call
+                                       * bus_message_unref_queued() for this which might then destroy the message
+                                       * while we are still processing it. */
         m->n_ref--;
 
-        if (m->n_ref > 0 || m->n_queued > 0)
+        if (m->n_ref > 0 || m->n_queued > 0) {
+                if (m->release_kdbus)
+                        sd_bus_unref(m->bus);
                 return NULL;
+        }
 
+        if (!m->release_kdbus)
         /* Unset the bus field if neither the user has a reference nor this message is queued. We are careful
          * to reset the field only after the last reference to the bus is dropped, after all we might keep
          * multiple references to the bus, once for each reference kept on outselves. */
-        m->bus = NULL;
+                m->bus = NULL;
 
         return message_free(m);
 }