daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
Pablo Greco 48fc63
From 0c4a78cc276249e51bcaa50b29df3efcf6d22ec3 Mon Sep 17 00:00:00 2001
Pablo Greco 48fc63
From: Jan Synacek <jsynacek@redhat.com>
Pablo Greco 48fc63
Date: Tue, 12 Feb 2019 11:10:59 +0100
Pablo Greco 48fc63
Subject: [PATCH] sd-bus: unify three code-paths which free struct
Pablo Greco 48fc63
 bus_container
Pablo Greco 48fc63
Pablo Greco 48fc63
We didn't free one of the fields in two of the places.
Pablo Greco 48fc63
Pablo Greco 48fc63
$ valgrind --show-leak-kinds=all --leak-check=full \
Pablo Greco 48fc63
  build/fuzz-bus-message \
Pablo Greco 48fc63
  test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20
Pablo Greco 48fc63
...
Pablo Greco 48fc63
==14457== HEAP SUMMARY:
Pablo Greco 48fc63
==14457==     in use at exit: 3 bytes in 1 blocks
Pablo Greco 48fc63
==14457==   total heap usage: 509 allocs, 508 frees, 51,016 bytes allocated
Pablo Greco 48fc63
==14457==
Pablo Greco 48fc63
==14457== 3 bytes in 1 blocks are definitely lost in loss record 1 of 1
Pablo Greco 48fc63
==14457==    at 0x4C2EBAB: malloc (vg_replace_malloc.c:299)
Pablo Greco 48fc63
==14457==    by 0x53AFE79: strndup (in /usr/lib64/libc-2.27.so)
Pablo Greco 48fc63
==14457==    by 0x4F52EB8: free_and_strndup (string-util.c:1039)
Pablo Greco 48fc63
==14457==    by 0x4F8E1AB: sd_bus_message_peek_type (bus-message.c:4193)
Pablo Greco 48fc63
==14457==    by 0x4F76CB5: bus_message_dump (bus-dump.c:144)
Pablo Greco 48fc63
==14457==    by 0x108F12: LLVMFuzzerTestOneInput (fuzz-bus-message.c:24)
Pablo Greco 48fc63
==14457==    by 0x1090F7: main (fuzz-main.c:34)
Pablo Greco 48fc63
==14457==
Pablo Greco 48fc63
==14457== LEAK SUMMARY:
Pablo Greco 48fc63
==14457==    definitely lost: 3 bytes in 1 blocks
Pablo Greco 48fc63
Pablo Greco 48fc63
(cherry picked from commit 6d1e0f4fcba8d6f425da3dc91805db95399b3c8b)
Pablo Greco 48fc63
Resolves: #1643394
Pablo Greco 48fc63
---
Pablo Greco 48fc63
 src/libsystemd/sd-bus/bus-message.c | 66 +++++++++++++++--------------
Pablo Greco 48fc63
 1 file changed, 35 insertions(+), 31 deletions(-)
Pablo Greco 48fc63
Pablo Greco 48fc63
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
Pablo Greco 48fc63
index 121e65674d..b786f943e4 100644
Pablo Greco 48fc63
--- a/src/libsystemd/sd-bus/bus-message.c
Pablo Greco 48fc63
+++ b/src/libsystemd/sd-bus/bus-message.c
Pablo Greco 48fc63
@@ -104,20 +104,42 @@ static void message_reset_parts(sd_bus_message *m) {
Pablo Greco 48fc63
         m->cached_rindex_part_begin = 0;
Pablo Greco 48fc63
 }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-static void message_reset_containers(sd_bus_message *m) {
Pablo Greco 48fc63
-        unsigned i;
Pablo Greco 48fc63
+static struct bus_container *message_get_container(sd_bus_message *m) {
Pablo Greco 48fc63
+        assert(m);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        if (m->n_containers == 0)
Pablo Greco 48fc63
+                return &m->root_container;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        assert(m->containers);
Pablo Greco 48fc63
+        return m->containers + m->n_containers - 1;
Pablo Greco 48fc63
+}
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+static void message_free_last_container(sd_bus_message *m) {
Pablo Greco 48fc63
+        struct bus_container *c;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+        c = message_get_container(m);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        free(c->signature);
Pablo Greco 48fc63
+        free(c->peeked_signature);
Pablo Greco 48fc63
+        free(c->offsets);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        /* Move to previous container, but not if we are on root container */
Pablo Greco 48fc63
+        if (m->n_containers > 0)
Pablo Greco 48fc63
+                m->n_containers--;
Pablo Greco 48fc63
+}
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+static void message_reset_containers(sd_bus_message *m) {
Pablo Greco 48fc63
         assert(m);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-        for (i = 0; i < m->n_containers; i++) {
Pablo Greco 48fc63
-                free(m->containers[i].signature);
Pablo Greco 48fc63
-                free(m->containers[i].offsets);
Pablo Greco 48fc63
-        }
Pablo Greco 48fc63
+        while (m->n_containers > 0)
Pablo Greco 48fc63
+                message_free_last_container(m);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         free(m->containers);
Pablo Greco 48fc63
         m->containers = NULL;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-        m->n_containers = m->containers_allocated = 0;
Pablo Greco 48fc63
+        m->containers_allocated = 0;
Pablo Greco 48fc63
         m->root_container.index = 0;
Pablo Greco 48fc63
 }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
@@ -151,10 +173,8 @@ static void message_free(sd_bus_message *m) {
Pablo Greco 48fc63
         }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         message_reset_containers(m);
Pablo Greco 48fc63
-        free(m->root_container.signature);
Pablo Greco 48fc63
-        free(m->root_container.offsets);
Pablo Greco 48fc63
-
Pablo Greco 48fc63
-        free(m->root_container.peeked_signature);
Pablo Greco 48fc63
+        assert(m->n_containers == 0);
Pablo Greco 48fc63
+        message_free_last_container(m);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         bus_creds_done(&m->creds);
Pablo Greco 48fc63
         free(m);
Pablo Greco 48fc63
@@ -1179,16 +1199,6 @@ _public_ int sd_bus_message_set_allow_interactive_authorization(sd_bus_message *
Pablo Greco 48fc63
         return 0;
Pablo Greco 48fc63
 }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-static struct bus_container *message_get_container(sd_bus_message *m) {
Pablo Greco 48fc63
-        assert(m);
Pablo Greco 48fc63
-
Pablo Greco 48fc63
-        if (m->n_containers == 0)
Pablo Greco 48fc63
-                return &m->root_container;
Pablo Greco 48fc63
-
Pablo Greco 48fc63
-        assert(m->containers);
Pablo Greco 48fc63
-        return m->containers + m->n_containers - 1;
Pablo Greco 48fc63
-}
Pablo Greco 48fc63
-
Pablo Greco 48fc63
 struct bus_body_part *message_append_part(sd_bus_message *m) {
Pablo Greco 48fc63
         struct bus_body_part *part;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
@@ -2107,6 +2117,7 @@ _public_ int sd_bus_message_open_container(
Pablo Greco 48fc63
         w = m->containers + m->n_containers++;
Pablo Greco 48fc63
         w->enclosing = type;
Pablo Greco 48fc63
         w->signature = signature;
Pablo Greco 48fc63
+        w->peeked_signature = NULL;
Pablo Greco 48fc63
         w->index = 0;
Pablo Greco 48fc63
         w->array_size = array_size;
Pablo Greco 48fc63
         w->before = before;
Pablo Greco 48fc63
@@ -4195,13 +4206,9 @@ _public_ int sd_bus_message_exit_container(sd_bus_message *m) {
Pablo Greco 48fc63
                         return -EBUSY;
Pablo Greco 48fc63
         }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-        free(c->signature);
Pablo Greco 48fc63
-        free(c->peeked_signature);
Pablo Greco 48fc63
-        free(c->offsets);
Pablo Greco 48fc63
-        m->n_containers--;
Pablo Greco 48fc63
+        message_free_last_container(m);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         c = message_get_container(m);
Pablo Greco 48fc63
-
Pablo Greco 48fc63
         saved = c->index;
Pablo Greco 48fc63
         c->index = c->saved_index;
Pablo Greco 48fc63
         r = container_next_item(m, c, &m->rindex);
Pablo Greco 48fc63
@@ -4219,16 +4226,13 @@ static void message_quit_container(sd_bus_message *m) {
Pablo Greco 48fc63
         assert(m->sealed);
Pablo Greco 48fc63
         assert(m->n_containers > 0);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
-        c = message_get_container(m);
Pablo Greco 48fc63
-
Pablo Greco 48fc63
         /* Undo seeks */
Pablo Greco 48fc63
+        c = message_get_container(m);
Pablo Greco 48fc63
         assert(m->rindex >= c->before);
Pablo Greco 48fc63
         m->rindex = c->before;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         /* Free container */
Pablo Greco 48fc63
-        free(c->signature);
Pablo Greco 48fc63
-        free(c->offsets);
Pablo Greco 48fc63
-        m->n_containers--;
Pablo Greco 48fc63
+        message_free_last_container(m);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         /* Correct index of new top-level container */
Pablo Greco 48fc63
         c = message_get_container(m);