[systemd-commits] 8 commits - src/import src/libsystemd-network src/machine src/network

David Herrmann dvdhrm at kemper.freedesktop.org
Wed Dec 31 07:30:21 PST 2014


 src/import/curl-util.c                 |    1 
 src/libsystemd-network/lldp-internal.c |    4 +-
 src/libsystemd-network/sd-lldp.c       |   51 ++++++++++-----------------------
 src/libsystemd-network/test-lldp.c     |    2 +
 src/machine/machinectl.c               |    3 -
 src/network/networkctl.c               |    2 -
 6 files changed, 22 insertions(+), 41 deletions(-)

New commits:
commit ee14ebf211df1322c3d8b550b931bbfa6cb3b033
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 16:28:48 2014 +0100

    lldp: fix sd_lldp_save()
    
    Fix a bunch of needless memzero() calls, a bunch of use-after-free
    regarding _cleanup_free_ and drop unused variables.
    
    Hint: Do NOT use _cleanup_free_ for temporary strappend() helpers that are
    freed multiple times. All you safe is the last free() call, which is
    really not worth the trouble resetting it to NULL all the time.

diff --git a/src/libsystemd-network/sd-lldp.c b/src/libsystemd-network/sd-lldp.c
index 74ea810..fa45310 100644
--- a/src/libsystemd-network/sd-lldp.c
+++ b/src/libsystemd-network/sd-lldp.c
@@ -429,7 +429,6 @@ static void lldp_mib_objects_flush(sd_lldp *lldp) {
 }
 
 int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
-        _cleanup_free_ char *s = NULL, *t = NULL, *k = NULL;
         _cleanup_free_ char *temp_path = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         uint8_t *mac, *port_id, type;
@@ -452,13 +451,13 @@ int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
 
         HASHMAP_FOREACH(c, lldp->neighbour_mib, i) {
                 LIST_FOREACH(port, p, c->ports) {
+                        _cleanup_free_ char *s = NULL;
+                        char *k, *t;
 
                         r = lldp_read_chassis_id(p->packet, &type, &length, &mac);
                         if (r < 0)
                                 continue;
 
-                        memzero(buf, LINE_MAX);
-
                         sprintf(buf, "'_Chassis=%02x:%02x:%02x:%02x:%02x:%02x' '_CType=%d' ",
                                 mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], type);
 
@@ -467,57 +466,43 @@ int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
                                 return -ENOMEM;
 
                         r = lldp_read_port_id(p->packet, &type, &length, &port_id);
-                        if (r < 0) {
-                                free(s);
+                        if (r < 0)
                                 continue;
-                        }
 
-                        memzero(buf, LINE_MAX);
                         if (type != LLDP_PORT_SUBTYPE_MAC_ADDRESS) {
-
                                 k = strndup((char *) port_id, length -1);
                                 if (!k)
                                         return -ENOMEM;
 
                                 sprintf(buf, "'_Port=%s' '_PType=%d' ", k , type);
-
-                                t = strappend(s, buf);
-
                                 free(k);
-                                k = NULL;
                         } else {
-
                                 mac = port_id;
-
                                 sprintf(buf, "'_Port=%02x:%02x:%02x:%02x:%02x:%02x' '_PType=%d' ",
                                         mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], type);
-
-                                t = strappend(s, buf);
                         }
 
-                        if (!t)
+                        k = strappend(s, buf);
+                        if (!k)
                                 return -ENOMEM;
 
                         free(s);
-                        s = t;
+                        s = k;
 
                         time = now(CLOCK_BOOTTIME);
 
                         /* Don't write expired packets */
-                        if(time - p->until <= 0) {
-                                free(s);
+                        if (time - p->until <= 0)
                                 continue;
-                        }
 
-                        memzero(buf, LINE_MAX);
                         sprintf(buf, "'_TTL=%lu' ", p->until);
 
-                        t = strappend(s, buf);
-                        if (!t)
+                        k = strappend(s, buf);
+                        if (!k)
                                 return -ENOMEM;
 
                         free(s);
-                        s = t;
+                        s = k;
 
                         r = lldp_read_system_name(p->packet, &length, &k);
                         if (r < 0)
@@ -528,6 +513,7 @@ int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
                                         return -ENOMEM;
 
                                 k = strjoin(s, "'_NAME=", t, "' ", NULL);
+                                free(t);
                         }
 
                         if (!k)
@@ -536,28 +522,22 @@ int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
                         free(s);
                         s = k;
 
-                        memzero(buf, LINE_MAX);
-
                         (void)lldp_read_system_capability(p->packet, &data);
 
                         sprintf(buf, "'_CAP=%x'", data);
 
-                        t = strappend(s, buf);
-                        if (!t)
+                        k = strappend(s, buf);
+                        if (!k)
                                 return -ENOMEM;
 
-                        fprintf(f, "%s\n", t);
-
                         free(s);
-                        free(t);
+                        s = k;
+
+                        fprintf(f, "%s\n", s);
                 }
         }
         r = 0;
 
-        s = NULL;
-        t = NULL;
-        k = NULL;
-
         fflush(f);
 
         if (ferror(f) || rename(temp_path, lldp_file) < 0) {

commit 7d4866548d028489d84c39af1bb9206842a77b2b
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 16:07:17 2014 +0100

    lldp: fix uninitialized cleanup var #2
    
    Another uninitialized variable marked as _cleanup_. Set it to NULL to
    avoid accessing uninitialized memory.

diff --git a/src/libsystemd-network/lldp-internal.c b/src/libsystemd-network/lldp-internal.c
index d03445b..f843fd2 100644
--- a/src/libsystemd-network/lldp-internal.c
+++ b/src/libsystemd-network/lldp-internal.c
@@ -496,7 +496,7 @@ int lldp_chassis_new(tlv_packet *tlv,
                      Prioq *by_expiry,
                      Hashmap *neighbour_mib,
                      lldp_chassis **ret) {
-        _cleanup_lldp_chassis_free_ lldp_chassis *c;
+        _cleanup_lldp_chassis_free_ lldp_chassis *c = NULL;
         uint16_t length;
         uint8_t *data;
         uint8_t type;

commit e7a2419a2ae2a8f56a3e2840f8d623d2a449277a
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 16:04:55 2014 +0100

    lldp: fix uninitialized cleanup var
    
    Make sure to set _cleanup_ variables to NULL. Otherwise, we free
    uninitialized objects.

diff --git a/src/libsystemd-network/lldp-internal.c b/src/libsystemd-network/lldp-internal.c
index f86c11e..d03445b 100644
--- a/src/libsystemd-network/lldp-internal.c
+++ b/src/libsystemd-network/lldp-internal.c
@@ -443,7 +443,7 @@ void lldp_neighbour_port_free(lldp_neighbour_port *p) {
 int lldp_neighbour_port_new(lldp_chassis *c,
                             tlv_packet *tlv,
                             lldp_neighbour_port **ret) {
-        _cleanup_lldp_neighbour_port_free_ lldp_neighbour_port *p;
+        _cleanup_lldp_neighbour_port_free_ lldp_neighbour_port *p = NULL;
         uint16_t length, ttl;
         uint8_t *data;
         uint8_t type;

commit c5285fbfcede2e0f54d2b5f14193041067cd2af6
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 16:01:37 2014 +0100

    import: fix mem-leak in CurlGlue
    
    Make sure to actually free the underlying object in CurlGlue unref.

diff --git a/src/import/curl-util.c b/src/import/curl-util.c
index 6a6b1c0..0c6c867 100644
--- a/src/import/curl-util.c
+++ b/src/import/curl-util.c
@@ -236,6 +236,7 @@ CurlGlue *curl_glue_unref(CurlGlue *g) {
 
         sd_event_source_unref(g->timer);
         sd_event_unref(g->event);
+        free(g);
 
         return NULL;
 }

commit 580e55da1118870b6099d1a863d9806a31f2b1b4
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 15:58:27 2014 +0100

    lldp: fix double free
    
    'k' is marked as _cleanup_free_ so reset it to NULL if we free it
    explicitly.

diff --git a/src/libsystemd-network/sd-lldp.c b/src/libsystemd-network/sd-lldp.c
index 08cd092..74ea810 100644
--- a/src/libsystemd-network/sd-lldp.c
+++ b/src/libsystemd-network/sd-lldp.c
@@ -484,6 +484,7 @@ int sd_lldp_save(sd_lldp *lldp, const char *lldp_file) {
                                 t = strappend(s, buf);
 
                                 free(k);
+                                k = NULL;
                         } else {
 
                                 mac = port_id;

commit fbee1d8587458922dec3fd8a9e0f663313697029
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 15:56:11 2014 +0100

    networkctl: fix strappend() error checking
    
    Make sure to test the right variable for NULL.

diff --git a/src/network/networkctl.c b/src/network/networkctl.c
index 21e9b38..15dfb81 100644
--- a/src/network/networkctl.c
+++ b/src/network/networkctl.c
@@ -880,7 +880,7 @@ static char *lldp_system_caps(uint16_t cap) {
         }
 
         t = strappend(s, "]");
-        if (!s)
+        if (!t)
                 return NULL;
 
         free(s);

commit 889cec8d58a00c8362ab0c6493bad39e9c6849bc
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 15:54:20 2014 +0100

    network: add malloc-assertion in test
    
    Make sure malloc() really returns non-NULL in lldp test.

diff --git a/src/libsystemd-network/test-lldp.c b/src/libsystemd-network/test-lldp.c
index e9d5d7b..288aac5 100644
--- a/src/libsystemd-network/test-lldp.c
+++ b/src/libsystemd-network/test-lldp.c
@@ -144,6 +144,7 @@ static int lldp_parse_port_id_tlv(tlv_packet *m) {
                 assert_se(tlv_packet_read_string(m, &str, &length) >= 0);
 
                 p = malloc0(length + 1);
+                assert_se(p);
                 strncpy(p, str, length-1);
 
                 assert_se(streq(p, TEST_LLDP_PORT) == 1);
@@ -182,6 +183,7 @@ static int lldp_parse_system_desc_tlv(tlv_packet *m) {
         assert_se(tlv_packet_read_string(m, &str, &length) >= 0);
 
         p = malloc0(length + 1);
+        assert_se(p);
         strncpy(p, str, length);
 
         assert_se(streq(p, TEST_LLDP_TYPE_SYSTEM_DESC) == 1);

commit 06a079055a68dd73150ed002fad7059ad3b41235
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Wed Dec 31 15:52:23 2014 +0100

    machinectl: remove dead code
    
    'r' is not touched after the previous error-checking 100 lines above. Drop
    that code.

diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 0abc251..b1d1707 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -250,9 +250,6 @@ static int list_images(int argc, char *argv[], void *userdata) {
                        (int) max_mtime, strna(format_timestamp(mtime_buf, sizeof(mtime_buf), images[j].mtime)));
         }
 
-        if (r < 0)
-                return bus_log_parse_error(r);
-
 
         if (arg_legend)
                 printf("\n%zu images listed.\n", n_images);



More information about the systemd-commits mailing list