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 672374 - Fix memory corruption in meta_later_remove
Fix memory corruption in meta_later_remove
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-03-19 05:01 UTC by Ray Strode [halfline]
Modified: 2012-03-19 06:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
util: Fix memory corruption in meta_later_remove (2.55 KB, patch)
2012-03-19 05:01 UTC, Ray Strode [halfline]
none Details | Review
util: Quit early once we've found and removed a later (756 bytes, patch)
2012-03-19 05:46 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Ray Strode [halfline] 2012-03-19 05:01:19 UTC
Downstream there's a number of people seeing crashers that
look to be heap corruption.  One user experiencing the problem
ran his session in valgrind and noticed a problem.

See https://bugzilla.redhat.com/show_bug.cgi?id=802903
Comment 1 Ray Strode [halfline] 2012-03-19 05:01:21 UTC
Created attachment 210070 [details] [review]
util: Fix memory corruption in meta_later_remove

commit 7f9472a58fe9605a63c0da37abe29f3c07ad54a9 plugged a memory
leak in meta_later_remove that was caused by the list node associated
with the later object never getting freed.

It introduced memory corruption, though, since the for loop immediately
accesses the now freed node to find the next node in the list.

This commit changes the for loop to a while loop and looks up the next
node before freeing the current node.

Spotted from valgrind log on downstream report:

https://bugzilla.redhat.com/show_bug.cgi?id=802903

==6045== Invalid read of size 8
==6045==    at 0x334C466190: meta_later_remove (util.c:914)
==6045==    by 0x334C466316: call_idle_later (util.c:834)
==6045==    by 0x3169444ACC: g_main_context_dispatch (gmain.c:2441)
==6045==    by 0x31694452C7: g_main_context_iterate (gmain.c:3089)
==6045==    by 0x3169445814: g_main_loop_run (gmain.c:3297)
==6045==    by 0x334C456AB0: meta_run (main.c:555)
==6045==    by 0x4029E0: main (main.c:571)
==6045==  Address 0x284d3d08 is 8 bytes inside a block of size 16
free'd
==6045==    at 0x4A0662E: free (vg_replace_malloc.c:366)
==6045==    by 0x316944B7E2: g_free (gmem.c:263)
==6045==    by 0x31694606BE: g_slice_free1 (gslice.c:907)
==6045==    by 0x3169461399: g_slist_delete_link (gslist.c:583)
==6045==    by 0x334C466148: meta_later_remove (util.c:919)
==6045==    by 0x334C466316: call_idle_later (util.c:834)
==6045==    by 0x3169444ACC: g_main_context_dispatch (gmain.c:2441)
==6045==    by 0x31694452C7: g_main_context_iterate (gmain.c:3089)
==6045==    by 0x3169445814: g_main_loop_run (gmain.c:3297)
==6045==    by 0x334C456AB0: meta_run (main.c:555)
==6045==    by 0x4029E0: main (main.c:571)
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-03-19 05:46:31 UTC
Created attachment 210073 [details] [review]
util: Quit early once we've found and removed a later

This prevents an invalid read and also improves performance slightly.



Alternate solution.
Comment 3 Ray Strode [halfline] 2012-03-19 05:47:41 UTC
Review of attachment 210073 [details] [review]:

yours is better.
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-03-19 06:00:25 UTC
Comment on attachment 210073 [details] [review]
util: Quit early once we've found and removed a later

Attachment 210073 [details] pushed as 5770b5b - util: Quit early once we've found and removed a later