[BlueZ,v2,1/2] mesh: Fix valgrind memory leaks

Message ID	20200516012742.573151-2-brian.gix@intel.com (mailing list archive)
State	Accepted
Headers	show Return-Path: <SRS0=p+wm=66=vger.kernel.org=linux-bluetooth-owner@kernel.org> IronPort-SDR: 0IFrDY+/Sf66Fhh2zpIfTvfGgwFutqR08DHg+vCNx+D+wfmT6G8r/z9iD/CnS0Wwo1lRn/b/Rg V+8iVET+vK0A== IronPort-SDR: 0T3vG1VZlL31+zCfhNSDIFmiFLrBEzm8TadnAXIdM6Gb930b88wEe9h+qHqCvYrgbXJmiYmSz0 P7M8nc2MoeRA== From: Brian Gix <brian.gix@intel.com> To: linux-bluetooth@vger.kernel.org Cc: inga.stotland@intel.com, brian.gix@intel.com Subject: [PATCH BlueZ v2 1/2] mesh: Fix valgrind memory leaks Date: Fri, 15 May 2020 18:27:41 -0700 Message-Id: <20200516012742.573151-2-brian.gix@intel.com> In-Reply-To: <20200516012742.573151-1-brian.gix@intel.com> References: <20200516012742.573151-1-brian.gix@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-bluetooth-owner@vger.kernel.org Precedence: bulk
Series	mesh: Valgrind Clean-up \| expand [BlueZ,v2,0/2] mesh: Valgrind Clean-up [BlueZ,v2,1/2] mesh: Fix valgrind memory leaks [BlueZ,v2,2/2] mesh: Fix valgrind memory leak warnings

Hi Brian, Sorry for the late response. On 05/15, Brian Gix wrote: > 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(-) > > diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c > index 9ac3979f8..6567d761c 100644 > --- a/mesh/mesh-config-json.c > +++ b/mesh/mesh-config-json.c > @@ -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); > diff --git a/mesh/mesh.c b/mesh/mesh.c > index 890a3aa8f..23ff9c2a8 100644 > --- a/mesh/mesh.c > +++ b/mesh/mesh.c > @@ -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, > diff --git a/mesh/node.c b/mesh/node.c > index 8914b639d..2b4b3a563 100644 > --- a/mesh/node.c > +++ b/mesh/node.c > @@ -335,6 +335,7 @@ static void free_node_resources(void *data) > > free_node_dbus_resources(node); > > + mesh_config_release(node->cfg); This introduces a double-free when application cancels a Join() call: 09:35:54.082 DBUS.in DEBUG / org.freedesktop.DBus.ObjectManager::GetManagedObjects() 09:35:54.082 MESHD.stderr INFO 2020-05-20 09:35:54.080 mesh.c:547 join_network_call mesh/mesh.c:join_network_call() Join network request 09:35:54.082 MESHD.stderr INFO 2020-05-20 09:35:54.080 node.c:1699 node_join mesh/node.c:node_join() 09:35:54.083 MESHD.stderr INFO 2020-05-20 09:35:54.082 node.c:1280 get_app_properties mesh/node.c:get_app_properties() path /com/silvair/application 09:35:54.083 MESHD.stderr INFO 2020-05-20 09:35:54.082 node.c:1127 get_element_properties mesh/node.c:get_element_properties() path /com/silvair/application/element0 09:35:54.083 MESHD.stderr INFO 2020-05-20 09:35:54.082 node.c:1127 get_element_properties mesh/node.c:get_element_properties() path /com/silvair/application/element1 09:35:54.084 MESHD.stderr INFO 2020-05-20 09:35:54.083 mesh-config-json.c:1706 mesh_config_create mesh/mesh-config-json.c:mesh_config_create() New node config /tmp/pytest-of-khorne/pytest-66/test_join_cancel0/config/83bf3b46810345a28bcce70d8d0b46d3/node.json 09:35:54.084 MESHD.stderr INFO 2020-05-20 09:35:54.083 mesh-io-tcpserver.c:773 tcpserver_io_reg tcpserver_io_reg 29 09:35:54.084 RADIO.out INFO LEAdvertisingDataTag.MESH_BEACON: 0083bf3b46810345a28bcce70d8d0b46d30000 09:35:54.085 DBUS.out DEBUG /org/bluez/mesh org.bluez.mesh.Network1::Join('/', b'\x83\xbf;F\x81\x03E\xa2\x8b\xcc\xe7\r\x8d\x0bF\xd3') = () 09:35:54.085 APPLICATION INFO Cancel 09:35:54.086 MESHD.stderr INFO 2020-05-20 09:35:54.085 mesh.c:592 cancel_join_call mesh/mesh.c:cancel_join_call() Cancel Join 09:35:54.086 MESHD.stderr INFO 2020-05-20 09:35:54.085 mesh-config-json.c:2327 mesh_config_destroy mesh/mesh-config-json.c:mesh_config_destroy() Delete node config /tmp/pytest-of-khorne/pytest-66/test_join_cancel0/config/83bf3b46810345a28bcce70d8d0b46d3 09:35:54.086 MESHD.stderr INFO 2020-05-20 09:35:54.086 util.c:146 del_fobject mesh/util.c:del_fobject() RM /tmp/pytest-of-khorne/pytest-66/test_join_cancel0/config/83bf3b46810345a28bcce70d8d0b46d3/node.json 09:35:54.086 MESHD.stderr INFO 2020-05-20 09:35:54.086 util.c:140 del_fobject mesh/util.c:del_fobject() RMDIR /tmp/pytest-of-khorne/pytest-66/test_join_cancel0/config/83bf3b46810345a28bcce70d8d0b46d3 09:35:54.086 MESHD.stderr INFO free(): double free detected in tcache 2 09:35:54.086 MESHD.segfault INFO *** Aborted 09:35:54.086 MESHD.registers DEBUG Register dump: 09:35:54.086 MESHD.registers DEBUG 09:35:54.090 MESHD.registers DEBUG RAX: 0000000000000000 RBX: 00007f6bc6ebf740 RCX: 00007f6bc6f333eb 09:35:54.090 MESHD.registers DEBUG RDX: 0000000000000000 RSI: 00007fffc10c1ef0 RDI: 0000000000000002 09:35:54.090 MESHD.registers DEBUG RBP: 00007fffc10c2240 R8 : 0000000000000000 R9 : 00007fffc10c1ef0 09:35:54.090 MESHD.registers DEBUG R10: 0000000000000008 R11: 0000000000000246 R12: 00007fffc10c2160 09:35:54.090 MESHD.registers DEBUG R13: 0000000000000010 R14: 00007f6bc74b0000 R15: 0000000000000002 09:35:54.090 MESHD.registers DEBUG RSP: 00007fffc10c1ef0 09:35:54.091 MESHD.registers DEBUG 09:35:54.091 MESHD.registers DEBUG RIP: 00007f6bc6f333eb EFLAGS: 00000246 09:35:54.091 MESHD.registers DEBUG 09:35:54.091 MESHD.registers DEBUG CS: 0033 FS: 0000 GS: 0000 09:35:54.091 MESHD.registers DEBUG 09:35:54.091 MESHD.registers DEBUG Trap: 00000000 Error: 00000000 OldMask: 00004002 CR2: 00000000 09:35:54.091 MESHD.registers DEBUG 09:35:54.091 MESHD.registers DEBUG FPUCW: 0000037f FPUSW: 00000000 TAG: 00000000 09:35:54.091 MESHD.registers DEBUG RIP: 00000000 RDP: 00000000 09:35:54.091 MESHD.registers DEBUG 09:35:54.091 MESHD.registers DEBUG ST(0) 0000 0000000000000000 ST(1) 0000 0000000000000000 09:35:54.092 MESHD.registers DEBUG ST(2) 0000 0000000000000000 ST(3) 0000 0000000000000000 09:35:54.092 MESHD.registers DEBUG ST(4) 0000 0000000000000000 ST(5) 0000 0000000000000000 09:35:54.092 MESHD.registers DEBUG ST(6) 0000 0000000000000000 ST(7) 0000 0000000000000000 09:35:54.092 MESHD.registers DEBUG mxcsr: 1f80 09:35:54.092 MESHD.registers DEBUG XMM0: 000000000000000000000000ffffffff XMM1: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM2: 000000000000000000000000ffffffff XMM3: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM4: 000000000000000000000000ffffffff XMM5: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM6: 000000000000000000000000ffffffff XMM7: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM8: 000000000000000000000000ffffffff XMM9: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM10: 000000000000000000000000ffffffff XMM11: 000000000000000000000000ffffffff 09:35:54.092 MESHD.registers DEBUG XMM12: 000000000000000000000000ffffffff XMM13: 000000000000000000000000ffffffff 09:35:54.093 MESHD.registers DEBUG XMM14: 000000000000000000000000ffffffff XMM15: 000000000000000000000000ffffffff 09:35:54.093 MESHD.registers DEBUG 09:35:54.093 MESHD.backtrace INFO Backtrace: 09:35:54.098 MESHD.backtrace INFO /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcb)[0x7f6bc6f333eb] 09:35:54.102 MESHD.backtrace INFO /lib/x86_64-linux-gnu/libc.so.6(abort+0x12b)[0x7f6bc6f12899] 09:35:54.175 MESHD.backtrace INFO __libc_message /build/glibc-t7JzpG/glibc-2.30/libio/../sysdeps/posix/libc_fatal.c:181 09:35:54.185 RADIO INFO Disconnected 09:35:54.186 APPLICATION ERROR Disconnected from org.bluez.mesh (:1.6564) 09:35:54.187 DBUS.out DEBUG /org/bluez/mesh org.bluez.mesh.Network1::Cancel() = ('Message recipient disconnected from message bus without replying',) ------------------------------------------------------------------------------------------------------------ Captured log teardown ------------------------------------------------------------------------------------------------------------ 09:35:54.294 MESHD WARNING Terminated with code: -6 09:35:54.297 MESHD.backtrace INFO thrd_yield /build/glibc-t7JzpG/glibc-2.30/malloc/malloc.c:5333 09:35:54.369 MESHD.backtrace INFO _int_free /build/glibc-t7JzpG/glibc-2.30/malloc/malloc.c:4201 09:35:54.375 MESHD.backtrace INFO l_queue_clear /home/khorne/Programming/silvair/bluez-tests/bluez/ell/queue.c:103 09:35:54.376 MESHD.backtrace DEBUG 101: entry = queue->head; 09:35:54.376 MESHD.backtrace DEBUG 102: 09:35:54.376 MESHD.backtrace DEBUG 103: while (entry) {

[BlueZ,v2,1/2] mesh: Fix valgrind memory leaks

Commit Message

Comments

Patch