After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 650840 - 3.0.2's memleak fix (commit b49e32) causes segfaults
3.0.2's memleak fix (commit b49e32) causes segfaults
Status: RESOLVED FIXED
Product: libgnome-keyring
Classification: Core
Component: General
3.0.x
Other Linux
: Normal major
: ---
Assigned To: GNOME keyring maintainer(s)
GNOME keyring maintainer(s)
: 651049 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-23 08:55 UTC by Martin Pitt
Modified: 2019-02-22 11:46 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
git formatted patch (1.26 KB, patch)
2011-05-23 09:02 UTC, Martin Pitt 		committed Details | Review

Description Martin Pitt 2011-05-23 08:55:38 UTC 
Hello,

After upgrading to libgnome-keyring 3.0.2 I noticed that nm-applet now crashes on startup:

+ Trace 227219

  • #0 raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #1 abort
    at abort.c line 92
  • #2 __libc_message
    at ../sysdeps/unix/sysv/linux/libc_fatal.c line 189
  • #3 malloc_printerr
  • #4 gnome_keyring_attribute_list_free
    at gnome-keyring-utils.c line 317
  • #5 gnome_keyring_find_itemsv_sync
    at gnome-keyring.c line 2523
  • #6 nm_gconf_migrate_0_7_keyring_items
    at gconf-upgrade.c line 1714 

The problem seems to be in the recent memory leak fix:
http://git.gnome.org/browse/libgnome-keyring/commit/?h=gnome-3-0&id=b49e32b32d4d7c326c3bd7dd44ca1c35944a94c4

gnome_keyring_find_itemsv_sync() now calls gnome_keyring_attribute_list_free() on the attribute list generated with make_attribute_list_va(). This is wrong, as the latter directly copies the (static) arguments from the caller, e. g. in nm-applet:


------------ 8< ------------
                ret = gnome_keyring_find_itemsv_sync (GNOME_KEYRING_ITEM_GENERIC_SECRET,
                                                      &found_list,
                                                      "connection-id",
                                                      GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
                                                      old_id,
                                                      NULL);

------------ 8< ------------

gnome_keyring_attribute_list_free() does not only free the actual array, but also all elements inside. It should perhaps just do a g_array_free() instead?

Note that this doesn't happen on trunk (3.1.1), as this memleak fix was _only_ applied to the 3-0 branch instead of master.
Comment 1 Martin Pitt 2011-05-23 09:02:42 UTC 
Created attachment 188361 [details] [review]
git formatted patch

This fixes the crash, and I think the logic is correct now.

This is also what make_attribute_list_va() does when it encounters an error.

Does that look ok to you, can I push it?

Do you want to handle the forward-porting of the memleak fix and this patch to master yourself, or want me to push that there, too?
Comment 2 Stef Walter 2011-05-23 11:24:54 UTC 
This looks good. Thanks. Yes, please go ahead and push. A similar patch was done on bug #650606 and tested there.

I can take care of merging the gnome-3-0 branch into master (in a few days).
Comment 3 Martin Pitt 2011-05-23 11:57:49 UTC 
Thanks, pushed.
Comment 4 Stef Walter 2011-05-28 17:36:36 UTC 
*** Bug 651077 has been marked as a duplicate of this bug. ***
Comment 5 Stef Walter 2011-05-28 17:38:19 UTC 
*** Bug 651049 has been marked as a duplicate of this bug. ***