From 5efa9ff34888171d8feb93890023af813c4fc5e1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 11 Feb 2019 17:54:13 +0100 Subject: [PATCH 1/5] shared: add nm_auto_decref_json --- shared/nm-utils/nm-jansson.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shared/nm-utils/nm-jansson.h b/shared/nm-utils/nm-jansson.h index 4f37ff46f..5a73231f0 100644 --- a/shared/nm-utils/nm-jansson.h +++ b/shared/nm-utils/nm-jansson.h @@ -41,6 +41,9 @@ n = json_object_iter_next(object, json_object_key_to_iter(key))) #endif +NM_AUTO_DEFINE_FCN0 (json_t *, _nm_auto_decref_json, json_decref) +#define nm_auto_decref_json nm_auto(_nm_auto_decref_json) + #endif /* WITH_JANSON */ #endif /* __NM_JANSSON_H__ */ From 8d9685ef98937a76b57c37c8d3f6946edfe1c420 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 11 Feb 2019 17:54:29 +0100 Subject: [PATCH 2/5] ovs: use nm_auto_decref_json --- src/devices/ovs/nm-ovsdb.c | 49 ++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 47cae7d99..8622f86bf 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -503,12 +503,15 @@ _add_interface (NMOvsdb *self, json_t *params, OpenvswitchBridge *ovs_bridge = NULL; OpenvswitchPort *ovs_port = NULL; OpenvswitchInterface *ovs_interface = NULL; + nm_auto_decref_json json_t *bridges = NULL; + nm_auto_decref_json json_t *new_bridges = NULL; + nm_auto_decref_json json_t *ports = NULL; + nm_auto_decref_json json_t *new_ports = NULL; + nm_auto_decref_json json_t *interfaces = NULL; + nm_auto_decref_json json_t *new_interfaces = NULL; + gboolean has_interface = FALSE; int pi; int ii; - json_t *bridges, *new_bridges; - json_t *ports, *new_ports; - json_t *interfaces, *new_interfaces; - gboolean has_interface = FALSE; bridges = json_array (); ports = json_array (); @@ -584,14 +587,6 @@ _add_interface (NMOvsdb *self, json_t *params, _insert_interface (params, interface); json_array_append_new (new_interfaces, json_pack ("[s, s]", "named-uuid", "rowInterface")); } - - json_decref (interfaces); - json_decref (ports); - json_decref (bridges); - - json_decref (new_interfaces); - json_decref (new_ports); - json_decref (new_bridges); } /** @@ -611,14 +606,12 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname) OpenvswitchBridge *ovs_bridge; OpenvswitchPort *ovs_port; OpenvswitchInterface *ovs_interface; - int pi; - int ii; json_t *bridges, *new_bridges; - json_t *ports, *new_ports; - json_t *interfaces, *new_interfaces; gboolean bridges_changed; gboolean ports_changed; gboolean interfaces_changed; + int pi; + int ii; bridges = json_array (); new_bridges = json_array (); @@ -626,20 +619,26 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname) g_hash_table_iter_init (&iter, priv->bridges); while (g_hash_table_iter_next (&iter, (gpointer) &bridge_uuid, (gpointer) &ovs_bridge)) { - json_array_append_new (bridges, json_pack ("[s,s]", "uuid", bridge_uuid)); + nm_auto_decref_json json_t *ports = NULL; + nm_auto_decref_json json_t *new_ports = NULL; ports = json_array (); new_ports = json_array (); ports_changed = FALSE; + json_array_append_new (bridges, json_pack ("[s,s]", "uuid", bridge_uuid)); + for (pi = 0; pi < ovs_bridge->ports->len; pi++) { + nm_auto_decref_json json_t *interfaces = NULL; + nm_auto_decref_json json_t *new_interfaces = NULL; + + interfaces = json_array (); + new_interfaces = json_array (); port_uuid = g_ptr_array_index (ovs_bridge->ports, pi); ovs_port = g_hash_table_lookup (priv->ports, port_uuid); json_array_append_new (ports, json_pack ("[s,s]", "uuid", port_uuid)); - interfaces = json_array (); - new_interfaces = json_array (); interfaces_changed = FALSE; for (ii = 0; ii < ovs_port->interfaces->len; ii++) { @@ -666,9 +665,6 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname) } json_array_append_new (new_ports, json_pack ("[s,s]", "uuid", port_uuid)); } - - json_decref (interfaces); - json_decref (new_interfaces); } if (json_array_size (new_ports) == 0) { @@ -680,9 +676,6 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname) } json_array_append_new (new_bridges, json_pack ("[s,s]", "uuid", bridge_uuid)); } - - json_decref (ports); - json_decref (new_ports); } if (bridges_changed) { @@ -707,7 +700,7 @@ ovsdb_next_command (NMOvsdb *self) NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self); OvsdbMethodCall *call = NULL; char *cmd; - json_t *msg = NULL; + nm_auto_decref_json json_t *msg = NULL; json_t *params; if (!priv->conn) @@ -763,7 +756,6 @@ ovsdb_next_command (NMOvsdb *self) cmd = json_dumps (msg, 0); g_string_append (priv->output, cmd); - json_decref (msg); free (cmd); ovsdb_write (self); @@ -1040,7 +1032,7 @@ static void ovsdb_got_echo (NMOvsdb *self, json_int_t id, json_t *data) { NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self); - json_t *msg; + nm_auto_decref_json json_t *msg = NULL; char *reply; gboolean output_was_empty; @@ -1049,7 +1041,6 @@ ovsdb_got_echo (NMOvsdb *self, json_int_t id, json_t *data) msg = json_pack ("{s:I, s:O}", "id", id, "result", data); reply = json_dumps (msg, 0); g_string_append (priv->output, reply); - json_decref (msg); free (reply); if (output_was_empty) From 139b9974fa34ee01b9ff57b45e64faaf645db5e8 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sun, 10 Feb 2019 13:32:28 +0100 Subject: [PATCH 3/5] ovs: fix leak of json objects Fixes: 830a5a14cb29ca00b73a9623c1ea7c5cd92f4d00 --- src/devices/ovs/nm-ovsdb.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 8622f86bf..d3f573ede 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -348,14 +348,14 @@ _insert_interface (json_t *params, NMConnection *interface) if (s_ovs_iface) type = nm_setting_ovs_interface_get_interface_type (s_ovs_iface); - json_array_append (options, json_string ("map")); + json_array_append_new (options, json_string ("map")); s_ovs_patch = nm_connection_get_setting_ovs_patch (interface); if (s_ovs_patch) { - json_array_append (options, json_pack ("[[s, s]]", + json_array_append_new (options, json_pack ("[[s, s]]", "peer", nm_setting_ovs_patch_get_peer (s_ovs_patch))); } else { - json_array_append (options, json_array ()); + json_array_append_new (options, json_array ()); } json_array_append_new (params, @@ -606,7 +606,8 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname) OpenvswitchBridge *ovs_bridge; OpenvswitchPort *ovs_port; OpenvswitchInterface *ovs_interface; - json_t *bridges, *new_bridges; + nm_auto_decref_json json_t *bridges = NULL; + nm_auto_decref_json json_t *new_bridges = NULL; gboolean bridges_changed; gboolean ports_changed; gboolean interfaces_changed; From b92f2c932310e0ab2c94af9824f2e683af773204 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sun, 10 Feb 2019 13:41:37 +0100 Subject: [PATCH 4/5] ovs: don't leak a GCancellable on connection failure Every time we clear priv->client we should also clear the cancellable or it will be leaked. Fixes: 830a5a14cb29ca00b73a9623c1ea7c5cd92f4d00 --- src/devices/ovs/nm-ovsdb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index d3f573ede..6fef75b61 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1308,6 +1308,7 @@ ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing) g_clear_object (&priv->client); g_clear_object (&priv->conn); g_clear_pointer (&priv->db_uuid, g_free); + nm_clear_g_cancellable (&priv->cancellable); } static void @@ -1540,9 +1541,6 @@ dispose (GObject *object) g_clear_pointer (&priv->ports, g_hash_table_destroy); g_clear_pointer (&priv->interfaces, g_hash_table_destroy); - g_cancellable_cancel (priv->cancellable); - g_clear_object (&priv->cancellable); - G_OBJECT_CLASS (nm_ovsdb_parent_class)->dispose (object); } From 29984c07cdd6ca00831bafc23ed7604abea7569e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 11 Feb 2019 17:54:48 +0100 Subject: [PATCH 5/5] ovs: fix dispose() input and output must be freed only when not NULL. Also, ovsdb_disconnect() should do nothing if there is no client. Fixes: 830a5a14cb29ca00b73a9623c1ea7c5cd92f4d00 --- src/devices/ovs/nm-ovsdb.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 6fef75b61..9d73c3ac7 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1291,6 +1291,9 @@ ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing) gpointer user_data; gs_free_error GError *error = NULL; + if (!priv->client) + return; + _LOGD ("disconnecting from ovsdb"); nm_utils_error_set_cancelled (&error, is_disposing, "NMOvsdb"); @@ -1527,11 +1530,14 @@ dispose (GObject *object) ovsdb_disconnect (self, TRUE); - g_string_free (priv->input, TRUE); - priv->input = NULL; - g_string_free (priv->output, TRUE); - priv->output = NULL; - + if (priv->input) { + g_string_free (priv->input, TRUE); + priv->input = NULL; + } + if (priv->output) { + g_string_free (priv->output, TRUE); + priv->output = NULL; + } if (priv->calls) { g_array_free (priv->calls, TRUE); priv->calls = NULL;