memory leak in nm_gconf_migrate_0_7_ca_cert_ignore

Bug #784756 reported by JKL
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
network-manager-applet (Ubuntu)
Fix Released
High
Mathieu Trudel-Lapierre

Bug Description

at line 1879 (with the quilt patchset applied), nm_gconf_migrate_0_7_ca_cert_ignore calls nm_gconf_get_string_helper, which causes memory to be allocated for the uuid parameter.

  if (!nm_gconf_get_string_helper (client, dir,
                                   NM_SETTING_CONNECTION_UUID,
                                   NM_SETTING_CONNECTION_SETTING_NAME,
                                   &uuid))

This memory is never freed. I believe the correct deallocation function here is g_free, because the allocation is done using g_strdup. Here is a valgrind log.

==10301== 333 bytes in 9 blocks are definitely lost in loss record 8,678 of 9,326
==10301== at 0x4C28FAC: malloc (vg_replace_malloc.c:236)
==10301== by 0x8F62A62: g_malloc (gmem.c:164)
==10301== by 0x8F7B06D: g_strdup (gstrfuncs.c:102)
==10301== by 0x43AF42: nm_gconf_get_string_helper (gconf-helpers.c:200)
==10301== by 0x443C83: nm_gconf_migrate_0_7_ca_cert_ignore (gconf-upgrade.c:1879)
==10301== by 0x43F85A: nm_gconf_get_all_connections (gconf-helpers.c:1694)
==10301== by 0x4465A5: read_connections (nma-gconf-settings.c:234)
==10301== by 0x44665E: list_connections (nma-gconf-settings.c:270)
==10301== by 0x52701D1: impl_settings_list_connections (nm-settings-service.c:107)
==10301== by 0x526FEBB: dbus_glib_marshal_nm_settings_BOOLEAN__POINTER_POINTER (nm-settings-glue.h:97)
==10301== by 0x6584C4C: ??? (in /usr/lib/libdbus-glib-1.so.2.1.0)
==10301== by 0x8475A00: _dbus_object_tree_dispatch_and_unlock (dbus-object-tree.c:858)
==10301== by 0x8467B0F: dbus_connection_dispatch (dbus-connection.c:4688)
==10301== by 0x6582654: ??? (in /usr/lib/libdbus-glib-1.so.2.1.0)
==10301== by 0x8F5BBCC: g_main_context_dispatch (gmain.c:2440)
==10301== by 0x8F5C3A7: g_main_context_iterate.clone.6 (gmain.c:3091)
==10301== by 0x8F5C9F1: g_main_loop_run (gmain.c:3299)
==10301== by 0x416D77: main (main.c:101)

I think the correct place to call g_free is at the very end of the block in which uuid is declared, but care must be taken that early outs are not introduced later. It would probably be a good idea to document the code block's responsibility to free the uuid.

Revision history for this message
JKL (jkl102001) wrote :
Changed in network-manager-applet (Ubuntu):
status: New → Fix Committed
Changed in network-manager-applet (Ubuntu):
status: Fix Committed → In Progress
importance: Undecided → High
assignee: nobody → Mathieu Trudel-Lapierre (mathieu-tl)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

This is in the master branch, so I'll upload all of this on Monday with the fixes for bug 724554 and bug 784711. (Friday afternoon isn't so great for uploading NetworkManager ;)

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Setting to Fix Committed since it's now in the packaging branch for nm-applet: lp:~network-manager/network-manager-applet/ubuntu.head

Changed in network-manager-applet (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package network-manager-applet - 0.8.9997+git.20110529t170033.9ec4c5d-0ubuntu1

---------------
network-manager-applet (0.8.9997+git.20110529t170033.9ec4c5d-0ubuntu1) oneiric; urgency=low

  * upstream snapshot 2011-05-29 17:00:33 (GMT)
    + 9ec4c5de855de5d9ee6c17752c3b0de6c68e9384
    - applet: fix leak in import/upgrade code (LP: #784756)
  * debian/rules: switch back to git "master" branch.
  * debian/patches/0001-applet-fix-possibly-uninitialized-variable.patch: drop,
    this is fixed upstream.
  * debian/patches/04-autostart.patch: refreshed.
  * debian/patches/20_use_full_vpn_dialog_service_name_path.patch: refreshed.
  * debian/patches/lp328572-dxteam-connect-text.patch: refreshed.
  * debian/patches/lp337960_dxteam_notification_icon_names.diff: refreshed.
  * debian/patches/lp341684_device_sensitive_disconnect_notify.patch: refresh.
  * debian/patches/lp460144_correctly_update_notification.patch: refreshed.
  * debian/patches/lp341684_device_sensitive_disconnect_notify.patch: refresh.
  * debian/patches/lp358526_generic_disconnected_notification_icon.patch:
    refreshed.
  * debian/patches/nm-applet-use-indicator.patch: refreshed and modified to
    build with GTK3 appindicator.
    - properly free icon_name for indicators (LP: #724554), thanks JKL.
    - fix leak in applet_menu_item_add_complex_separator_helper due to new'ing
      a GtkSeparatorMenuItem on top of a GtkImageMenuItem (LP: #784711).
  * debian/control:
    - Bump Build-Depends to libappindicator3-dev.
    - Update Build-Depends for GTK to libgtk-3-dev.
    - Bump network-manager and libnm Depends and Build-Depends to 0.8.998.
  * debian/rules, debian/patches/series: pass --libexecdir to configure, which
    now renders the patch 20_use_full_vpn_dialog_service_name_path.patch
    unnecessary, so we're dropping it.
  * debian/patches/20_use_full_vpn_dialog_service_name_path.patch: dropped,
    see above.
 -- Mathieu Trudel-Lapierre <email address hidden> Mon, 30 May 2011 13:25:18 -0400

Changed in network-manager-applet (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.