daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
36e8a3
From eb141ba81158feb74118da4e7a3f2266b11ffe10 Mon Sep 17 00:00:00 2001
36e8a3
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
36e8a3
Date: Mon, 9 Jul 2018 08:06:28 +0200
36e8a3
Subject: [PATCH] sd-bus: unify three code-paths which free struct
36e8a3
 bus_container
36e8a3
36e8a3
We didn't free one of the fields in two of the places.
36e8a3
36e8a3
$ valgrind --show-leak-kinds=all --leak-check=full \
36e8a3
  build/fuzz-bus-message \
36e8a3
  test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20
36e8a3
...
36e8a3
==14457== HEAP SUMMARY:
36e8a3
==14457==     in use at exit: 3 bytes in 1 blocks
36e8a3
==14457==   total heap usage: 509 allocs, 508 frees, 51,016 bytes allocated
36e8a3
==14457==
36e8a3
==14457== 3 bytes in 1 blocks are definitely lost in loss record 1 of 1
36e8a3
==14457==    at 0x4C2EBAB: malloc (vg_replace_malloc.c:299)
36e8a3
==14457==    by 0x53AFE79: strndup (in /usr/lib64/libc-2.27.so)
36e8a3
==14457==    by 0x4F52EB8: free_and_strndup (string-util.c:1039)
36e8a3
==14457==    by 0x4F8E1AB: sd_bus_message_peek_type (bus-message.c:4193)
36e8a3
==14457==    by 0x4F76CB5: bus_message_dump (bus-dump.c:144)
36e8a3
==14457==    by 0x108F12: LLVMFuzzerTestOneInput (fuzz-bus-message.c:24)
36e8a3
==14457==    by 0x1090F7: main (fuzz-main.c:34)
36e8a3
==14457==
36e8a3
==14457== LEAK SUMMARY:
36e8a3
==14457==    definitely lost: 3 bytes in 1 blocks
36e8a3
36e8a3
(cherry picked from commit 6d1e0f4fcba8d6f425da3dc91805db95399b3c8b)
36e8a3
Resolves: #1635435
36e8a3
---
36e8a3
 src/libsystemd/sd-bus/bus-message.c           |  64 +++++++++---------
36e8a3
 ...k-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 | Bin 0 -> 534 bytes
36e8a3
 2 files changed, 32 insertions(+), 32 deletions(-)
36e8a3
 create mode 100644 test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20
36e8a3
36e8a3
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
4bff0a
index 7c8bad2bdd..d55cb14843 100644
36e8a3
--- a/src/libsystemd/sd-bus/bus-message.c
36e8a3
+++ b/src/libsystemd/sd-bus/bus-message.c
36e8a3
@@ -77,19 +77,38 @@ static void message_reset_parts(sd_bus_message *m) {
36e8a3
         m->cached_rindex_part_begin = 0;
36e8a3
 }
36e8a3
 
36e8a3
-static void message_reset_containers(sd_bus_message *m) {
36e8a3
-        unsigned i;
36e8a3
+static struct bus_container *message_get_container(sd_bus_message *m) {
36e8a3
+        assert(m);
36e8a3
+
36e8a3
+        if (m->n_containers == 0)
36e8a3
+                return &m->root_container;
36e8a3
+
36e8a3
+        assert(m->containers);
36e8a3
+        return m->containers + m->n_containers - 1;
36e8a3
+}
36e8a3
+
36e8a3
+static void message_free_last_container(sd_bus_message *m) {
36e8a3
+        struct bus_container *c;
36e8a3
+
36e8a3
+        c = message_get_container(m);
36e8a3
+
36e8a3
+        free(c->signature);
36e8a3
+        free(c->peeked_signature);
36e8a3
+        free(c->offsets);
36e8a3
+
36e8a3
+        /* Move to previous container, but not if we are on root container */
36e8a3
+        if (m->n_containers > 0)
36e8a3
+                m->n_containers--;
36e8a3
+}
36e8a3
 
36e8a3
+static void message_reset_containers(sd_bus_message *m) {
36e8a3
         assert(m);
36e8a3
 
36e8a3
-        for (i = 0; i < m->n_containers; i++) {
36e8a3
-                free(m->containers[i].signature);
36e8a3
-                free(m->containers[i].offsets);
36e8a3
-        }
36e8a3
+        while (m->n_containers > 0)
36e8a3
+                message_free_last_container(m);
36e8a3
 
36e8a3
         m->containers = mfree(m->containers);
36e8a3
-
36e8a3
-        m->n_containers = m->containers_allocated = 0;
36e8a3
+        m->containers_allocated = 0;
36e8a3
         m->root_container.index = 0;
36e8a3
 }
36e8a3
 
36e8a3
@@ -112,10 +131,8 @@ static sd_bus_message* message_free(sd_bus_message *m) {
36e8a3
                 free(m->iovec);
36e8a3
 
36e8a3
         message_reset_containers(m);
36e8a3
-        free(m->root_container.signature);
36e8a3
-        free(m->root_container.offsets);
36e8a3
-
36e8a3
-        free(m->root_container.peeked_signature);
36e8a3
+        assert(m->n_containers == 0);
36e8a3
+        message_free_last_container(m);
36e8a3
 
36e8a3
         bus_creds_done(&m->creds);
36e8a3
         return mfree(m);
36e8a3
@@ -1113,16 +1130,6 @@ _public_ int sd_bus_message_set_allow_interactive_authorization(sd_bus_message *
36e8a3
         return 0;
36e8a3
 }
36e8a3
 
36e8a3
-static struct bus_container *message_get_container(sd_bus_message *m) {
36e8a3
-        assert(m);
36e8a3
-
36e8a3
-        if (m->n_containers == 0)
36e8a3
-                return &m->root_container;
36e8a3
-
36e8a3
-        assert(m->containers);
36e8a3
-        return m->containers + m->n_containers - 1;
36e8a3
-}
36e8a3
-
36e8a3
 struct bus_body_part *message_append_part(sd_bus_message *m) {
36e8a3
         struct bus_body_part *part;
36e8a3
 
36e8a3
@@ -4108,13 +4115,9 @@ _public_ int sd_bus_message_exit_container(sd_bus_message *m) {
36e8a3
                         return -EBUSY;
36e8a3
         }
36e8a3
 
36e8a3
-        free(c->signature);
36e8a3
-        free(c->peeked_signature);
36e8a3
-        free(c->offsets);
36e8a3
-        m->n_containers--;
36e8a3
+        message_free_last_container(m);
36e8a3
 
36e8a3
         c = message_get_container(m);
36e8a3
-
36e8a3
         saved = c->index;
36e8a3
         c->index = c->saved_index;
36e8a3
         r = container_next_item(m, c, &m->rindex);
36e8a3
@@ -4132,16 +4135,13 @@ static void message_quit_container(sd_bus_message *m) {
36e8a3
         assert(m->sealed);
36e8a3
         assert(m->n_containers > 0);
36e8a3
 
36e8a3
-        c = message_get_container(m);
36e8a3
-
36e8a3
         /* Undo seeks */
36e8a3
+        c = message_get_container(m);
36e8a3
         assert(m->rindex >= c->before);
36e8a3
         m->rindex = c->before;
36e8a3
 
36e8a3
         /* Free container */
36e8a3
-        free(c->signature);
36e8a3
-        free(c->offsets);
36e8a3
-        m->n_containers--;
36e8a3
+        message_free_last_container(m);
36e8a3
 
36e8a3
         /* Correct index of new top-level container */
36e8a3
         c = message_get_container(m);
36e8a3
diff --git a/test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 b/test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20
36e8a3
new file mode 100644
36e8a3
index 0000000000000000000000000000000000000000..c371824ffb604708619fd0713e8fca609bac18f7
36e8a3
GIT binary patch
36e8a3
literal 534
36e8a3
zcmZ{h!A`?442GSJP20o?A&zJgm*%p
36e8a3
z_LnqH{-ic!J`GWMLG(>T#&`l!4rxq{&>8YmwQrOs;B(}I_m11m8`nFp
36e8a3
z!cs!Q@A35`W+B>`#ek1>oQYVSs`!XH?7Y=}3y9Ye+UliL9^x9s66$8wH+TPdOG`n|
36e8a3
z5Uhx
36e8a3
zLhpD3X)k6@tX`CzbBVV-7e$fy9()CjJ&n(=^)uJCKFB5Xi}-<1ru7po5XlEJ?uByQ
36e8a3
MaEPzRhwknF02{PjtN;K2
36e8a3
36e8a3
literal 0
36e8a3
HcmV?d00001
36e8a3