mission-control-5 assert failure: *** glibc detected *** /usr/lib/telepathy/mission-control-5: double free or corruption (out): 0x0000000001756790 ***

Bug #1024848 reported by Anders Kaseorg
68
This bug affects 10 people
Affects Status Importance Assigned to Milestone
Telepathy Mission Control 5
Fix Released
Medium
telepathy-mission-control-5 (Ubuntu)
Fix Released
High
Unassigned
Precise
Fix Released
High
Unassigned

Bug Description

Impact:
the telepathy-mission-control-5 service regularly segfaults due to invalid frees

Test Case:
no real testcase, that's a corruption bug and not easy to trigger, just check that empathy keeps working normally

Regression testing:
telepathy-mission-control is used by empathy, try adding an account and using it

Revision history for this message
In , Laurent Bigonville (bigon) wrote :

Created attachment 63953
Valgrind

Hi,

I've several crash that are affecting me quite often (bug #51559 and #51813)

I was running MC in valgrind and it seems it tries to access freed memory in several occasions

Revision history for this message
In , Xavier Claessens (zdra) wrote :

*** Bug 51454 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Xavier Claessens (zdra) wrote :

I've understood how this can happen:
in mcd-account-manager-default.c::_delete_from_keyring() it does:
      gchar *removed = g_strdup (account);
      g_hash_table_insert (amd->removed_accounts, removed, removed);

So what happens if the account is already in the hash table? Doc says:
"""
If the key already exists in the GHashTable its current value is replaced with the new value. If you supplied a value_destroy_func when creating the GHashTable, the old value is freed using that function. If you supplied a key_destroy_func when creating the GHashTable, the passed key is freed using that function.
"""

Since @removed is passed both as value and key, it gets inserted as value but freed as key => we get a freed string as value!!

Revision history for this message
In , Xavier Claessens (zdra) wrote :

Created attachment 64015
McdAccountManagerDefault: Fix a possible double free

If the account is already in the hashtable, g_hash_table_insert()
will set @removed as value, but free it since the key already in
the table is kept.

Revision history for this message
In , Xavier Claessens (zdra) wrote :

Note that we could use g_hash_table_add() but that's new in glib 2.32 and MC still depends on 2.30. In master that's certainly OK, but probably not for backporting in stable branch.

Revision history for this message
In , Jonny-lamb (jonny-lamb) wrote :

Can you write a test for this perchance?

Revision history for this message
In , Laurent Bigonville (bigon) wrote :

Doesn't it sound weird that MC is actually calling _delete_from_keyring() function from mcd_account_connection_self_nickname_changed_cb() ?

Revision history for this message
In , Xavier Claessens (zdra) wrote :

Right, that looks suspicious. Also I'm starting to wonder if I really fixed this issue.

As explained above, we can end with freed strings as value in the hash table, but the value is never used... only the key is. And I don't see how the key could be invalid.

Revision history for this message
In , Xavier Claessens (zdra) wrote :

Ok, it continuously wants to remove *GOA* accounts from default storage (keyring and .mission-control/accounts/accounts.cfg), because they are not stored there.

My patch really fix the issue, I was able reproduce the valgrind logs and with the patch I can't anymore.

I've absolutely no idea how to test this behaviour though, that involves having accounts on other storage than the default one... and even then you need valgrind to catch the problem, it does not always crash.

Revision history for this message
In , Jonny-lamb (jonny-lamb) wrote :

MC already has a test account storage -- the diverted plugin.

All accounts will be saved in the non-default account storage unless the account parameter starts with 'dontdivert'.

Revision history for this message
In , Xavier Claessens (zdra) wrote :

I couldn't reproduce this in a unit test. I've spent enough time on this 1 line fix, if someone knows how MC tests works please do it :-)

Revision history for this message
Anders Kaseorg (andersk) wrote :
Revision history for this message
Apport retracing service (apport) wrote : This bug is a duplicate

Thank you for taking the time to report this crash and helping to make this software better. This particular crash has already been reported and is a duplicate of bug #984450, so is being marked as such. Please look at the other bug report to see if there is any missing information that you can provide, or to see if there is a workaround for the bug. Additionally, any further discussion regarding the bug should occur in the other report. Please continue to report any other bugs you may find.

visibility: private → public
tags: removed: need-amd64-retrace
Revision history for this message
Anders Kaseorg (andersk) wrote :

Argh, private bugs aren’t useful to anyone; undupeing. This one looks like it has an upstream patch.

Changed in mission-control-5:
importance: Unknown → Medium
status: Unknown → Fix Released
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thank you for your bug report, it could well be https://bugs.freedesktop.org/show_bug.cgi?id=51842 which got fixed with 5.13 in quantal, going to backport the fix for precise

Changed in telepathy-mission-control-5 (Ubuntu):
importance: Undecided → High
status: New → Fix Released
Changed in telepathy-mission-control-5 (Ubuntu Precise):
importance: Undecided → High
status: New → In Progress
description: updated
tags: removed: apparmor
Revision history for this message
Scott Kitterman (kitterman) wrote : Please test proposed package

Hello Anders, or anyone else affected,

Accepted into precise-proposed. The package will build now and be available in a few hours in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in telepathy-mission-control-5 (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Steve Langasek (vorlon) wrote :

This is probably the same as bug #984475 (i386) and bug #984450 (amd64). errors.ubuntu.com reports no instances of either of these bugs with the SRU version:

https://errors.ubuntu.com/bucket/?id=/usr/lib/telepathy/mission-control-5:***%20glibc%20detected%20***%20/usr/lib/telepathy/mission-control-5:%20double%20free%20or%20corruption%20%28out%29:%20ADDR%20***

https://errors.ubuntu.com/bucket/?id=/usr/lib/telepathy/mission-control-5:***%20glibc%20detected%20***%20/usr/lib/telepathy/mission-control-5:%20munmap_chunk():%20invalid%20pointer:%20ADDR%20***

This is moderate evidence that the bug may be fixed, however the bug is not so common that the lack of reports from precise-proposed makes this a certainty.

It would be helpful if users who have seen this bug would install the version of the telepathy-mission-control-5 packages from precise-proposed, and confirm whether it solves the problem for them.

Revision history for this message
Sebastien Bacher (seb128) wrote :

the code change is logical, no regression using the new version here and Steve's comment suggest that the update improved thing, even if we are not sure the update is fixing that specific bug it's a nice improvement and stops random segfaults so marking it verified

tags: added: verification-done
removed: verification-needed
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package telepathy-mission-control-5 - 1:5.12.0-0ubuntu2.1

---------------
telepathy-mission-control-5 (1:5.12.0-0ubuntu2.1) precise-proposed; urgency=low

  * debian/patches/git_no_double_free.patch:
    - backport upstream fix for double free bug (lp: #1024848)
 -- Sebastien Bacher <email address hidden> Sat, 28 Jul 2012 18:37:21 +0200

Changed in telepathy-mission-control-5 (Ubuntu Precise):
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.