mesh: Valgrind Clean-up

PATCH Superseded Brian Gix 2 participants 4 messages May 15, 2020 – May 16, 2020 Bluetooth drivers

Share & tools

Brian Gix

author

2020-05-15 23:59 UTC #0 lore raw

cover mesh: Valgrind Clean-up

These two patches address all known outstanding valgrind issues with the mesh daemon. The first patch (1/2) fixes actual memory leaks that will compound over time. The second patch (2/2) fixes less critical warnings that some memory wasn't entirely freed before exiting.

Brian Gix (2):
mesh: Fix valgrind memory leaks
mesh: Fix valgrind memory leak warnings

 mesh/agent.c            |  1 +
 mesh/mesh-config-json.c | 16 ++++++++--------
 mesh/mesh.c             |  9 ++++++++-
 mesh/net-keys.c         |  6 ++++++
 mesh/net-keys.h         |  1 +
 mesh/net.c              | 12 +++++++++++-
 mesh/net.h              |  3 ++-
 mesh/node.c             |  1 +
 8 files changed, 38 insertions(+), 11 deletions(-)

Signature · 1 line

2.25.4

Brian Gix

author

2020-05-15 23:59 UTC #1 lore raw

PATCH 1/2 mesh: Fix valgrind memory leaks

These memory leaks are ones that will compound over time with node creation and deletion.

---
 mesh/mesh-config-json.c | 16 ++++++++--------
 mesh/mesh.c             |  5 ++++-
 mesh/node.c             |  1 +
 3 files changed, 13 insertions(+), 9 deletions(-)

mesh/mesh-config-json.c +8 -8

index 9ac3979f8..6567d761c 100644
@@ -447,8 +447,6 @@ static bool read_app_keys(json_object *jobj, struct mesh_config_node *node)
 	if (!len)
 		return true;
 
-	node->appkeys = l_queue_new();
-
 	for (i = 0; i < len; ++i) {
 		json_object *jtemp, *jvalue;
 		char *str;

@@ -505,8 +503,6 @@ static bool read_net_keys(json_object *jobj, struct mesh_config_node *node)
 	if (!len)
 		return false;
 
-	node->netkeys = l_queue_new();
-
 	for (i = 0; i < len; ++i) {
 		json_object *jtemp, *jvalue;
 		char *str;

@@ -1133,8 +1129,6 @@ static bool parse_elements(json_object *jelems, struct mesh_config_node *node)
 		/* Allow "empty" nodes */
 		return true;
 
-	node->elements = l_queue_new();
-
 	for (i = 0; i < num_ele; ++i) {
 		json_object *jelement;
 		json_object *jmodels;

@@ -1154,6 +1148,7 @@ static bool parse_elements(json_object *jelems, struct mesh_config_node *node)
 		ele = l_new(struct mesh_config_element, 1);
 		ele->index = index;
 		ele->models = l_queue_new();
+		l_queue_push_tail(node->elements, ele);
 
 		if (!json_object_object_get_ex(jelement, "location", &jvalue))
 			goto fail;

@@ -1167,8 +1162,6 @@ static bool parse_elements(json_object *jelems, struct mesh_config_node *node)
 						!parse_models(jmodels, ele))
 				goto fail;
 		}
-
-		l_queue_push_tail(node->elements, ele);
 	}
 
 	return true;

@@ -2133,6 +2126,11 @@ static bool load_node(const char *fname, const uint8_t uuid[16],
 		goto done;
 
 	memset(&node, 0, sizeof(node));
+
+	node.elements = l_queue_new();
+	node.netkeys = l_queue_new();
+	node.appkeys = l_queue_new();
+
 	result = read_node(jnode, &node);
 
 	if (result) {

@@ -2148,6 +2146,7 @@ static bool load_node(const char *fname, const uint8_t uuid[16],
 		result = cb(&node, uuid, cfg, user_data);
 
 		if (!result) {
+			l_free(cfg->idles);
 			l_free(cfg->node_dir_path);
 			l_free(cfg);
 		}

@@ -2157,6 +2156,7 @@ static bool load_node(const char *fname, const uint8_t uuid[16],
 	l_free(node.net_transmit);
 	l_queue_destroy(node.netkeys, l_free);
 	l_queue_destroy(node.appkeys, l_free);
+	l_queue_destroy(node.elements, free_element);
 
 	if (!result)
 		json_object_put(jnode);

mesh/mesh.c +4 -1

index 890a3aa8f..23ff9c2a8 100644
@@ -209,7 +209,7 @@ static void parse_settings(const char *mesh_conf_fname)
 
 	settings = l_settings_new();
 	if (!l_settings_load_from_file(settings, mesh_conf_fname))
-		return;
+		goto done;
 
 	str = l_settings_get_string(settings, "General", "Beacon");
 	if (str) {

@@ -242,6 +242,9 @@ static void parse_settings(const char *mesh_conf_fname)
 
 	if (l_settings_get_uint(settings, "General", "ProvTimeout", &value))
 		mesh.prov_timeout = value;
+
+done:
+	l_settings_free(settings);
 }
 
 bool mesh_init(const char *config_dir, const char *mesh_conf_fname,

mesh/node.c +1 -0

index 581899562..382997a4c 100644
@@ -336,6 +336,7 @@ static void free_node_resources(void *data)
 
 	free_node_dbus_resources(node);
 
+	mesh_config_release(node->cfg);
 	mesh_net_free(node->net);
 	l_free(node->storage_dir);
 	l_free(node);

Signature · 1 line

2.25.4

Brian Gix

author

2020-05-15 23:59 UTC #2 lore raw

PATCH 2/2 mesh: Fix valgrind memory leak warnings

In reply to Brian Gix

These warnings are caused by not completely freeing memory allocations a shutdown, and are not serious, but they make valgrind output cleaner.

---
 mesh/agent.c    |  1 +
 mesh/mesh.c     |  4 ++++
 mesh/net-keys.c |  6 ++++++
 mesh/net-keys.h |  1 +
 mesh/net.c      | 12 +++++++++++-
 mesh/net.h      |  3 ++-
 6 files changed, 25 insertions(+), 2 deletions(-)

mesh/agent.c +1 -0

index bb52f4146..a06cc2b99 100644
@@ -245,6 +245,7 @@ void mesh_agent_cleanup(void)
 		return;
 
 	l_queue_destroy(agents, agent_free);
+	agents = NULL;
 
 }

mesh/mesh.c +4 -0

index 23ff9c2a8..451cefbb4 100644
@@ -27,6 +27,7 @@
 #include "mesh/mesh-io.h"
 #include "mesh/node.h"
 #include "mesh/net.h"
+#include "mesh/net-keys.h"
 #include "mesh/provision.h"
 #include "mesh/model.h"
 #include "mesh/dbus.h"

@@ -340,8 +341,11 @@ void mesh_cleanup(void)
 	}
 
 	l_queue_destroy(pending_queue, pending_request_exit);
+	mesh_agent_cleanup();
 	node_cleanup_all();
 	mesh_model_cleanup();
+	mesh_net_cleanup();
+	net_key_cleanup();
 
 	l_dbus_object_remove_interface(dbus_get_bus(), BLUEZ_MESH_PATH,
 							MESH_NETWORK_INTERFACE);

mesh/net-keys.c +6 -0

index f7eb2ca68..409ecfd08 100644
@@ -523,3 +523,9 @@ void net_key_beacon_disable(uint32_t id)
 	l_timeout_remove(key->snb.timeout);
 	key->snb.timeout = NULL;
 }
+
+void net_key_cleanup()
+{
+	l_queue_destroy(keys, l_free);
+	keys = NULL;
+}

mesh/net-keys.h +1 -0

index 9385e2c51..4f480fcda 100644
@@ -21,6 +21,7 @@
 #define KEY_REFRESH		0x01
 #define IV_INDEX_UPDATE		0x02
 
+void net_key_cleanup(void);
 bool net_key_confirm(uint32_t id, const uint8_t master[16]);
 bool net_key_retrieve(uint32_t id, uint8_t *master);
 uint32_t net_key_add(const uint8_t master[16]);

mesh/net.c +11 -1

index bfb9c4435..10a7c4616 100644
@@ -681,8 +681,10 @@ struct mesh_net *mesh_net_new(struct mesh_node *node)
 	return net;
 }
 
-void mesh_net_free(struct mesh_net *net)
+void mesh_net_free(void *user_data)
 {
+	struct mesh_net *net = user_data;
+
 	if (!net)
 		return;

@@ -701,6 +703,14 @@ void mesh_net_free(struct mesh_net *net)
 	l_free(net);
 }
 
+void mesh_net_cleanup()
+{
+	l_queue_destroy(fast_cache, l_free);
+	fast_cache = NULL;
+	l_queue_destroy(nets, mesh_net_free);
+	nets = NULL;
+}
+
 bool mesh_net_set_seq_num(struct mesh_net *net, uint32_t seq)
 {
 	if (!net)

mesh/net.h +2 -1

index bfc8064f3..8646d5aef 100644
@@ -265,7 +265,8 @@ typedef void (*mesh_net_status_func_t)(uint16_t remote, uint8_t status,
 					void *user_data);
 
 struct mesh_net *mesh_net_new(struct mesh_node *node);
-void mesh_net_free(struct mesh_net *net);
+void mesh_net_free(void *net);
+void mesh_net_cleanup(void);
 void mesh_net_flush_msg_queues(struct mesh_net *net);
 void mesh_net_set_iv_index(struct mesh_net *net, uint32_t index, bool update);
 bool mesh_net_iv_index_update(struct mesh_net *net);

Signature · 1 line

2.25.4

[email protected]

2020-05-16 00:09 UTC #3 lore raw

RE: [BlueZ,2/2] mesh: Fix valgrind memory leak warnings

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. While we are preparing for reviewing the patches, we found the following issue/warning.

Test Result:
checkpatch Failed

Outputs:
ERROR:FUNCTION_WITHOUT_ARGS: Bad function definition - void net_key_cleanup() should probably be void net_key_cleanup(void) #54: FILE: mesh/net-keys.c:527:
+void net_key_cleanup()

ERROR:FUNCTION_WITHOUT_ARGS: Bad function definition - void mesh_net_cleanup() should probably be void mesh_net_cleanup(void) #91: FILE: mesh/net.c:706:
+void mesh_net_cleanup()

- total: 2 errors, 0 warnings, 75 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

---
Regards,
Linux Bluetooth