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 781658 - atk-adaptor/bridge: Fix GList handling resulting in memory corruption
atk-adaptor/bridge: Fix GList handling resulting in memory corruption
Status: RESOLVED FIXED
Product: at-spi
Classification: Platform
Component: at-spi2-atk
unspecified
Other All
: Normal normal
: ---
Assigned To: At-spi maintainer(s)
At-spi maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-04-24 12:52 UTC by Rui Matos
Modified: 2017-04-25 15:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
atk-adaptor/bridge: Fix GList handling resulting in memory corruption (4.57 KB, patch)
2017-04-24 12:52 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2017-04-24 12:52:15 UTC
See patch.
Comment 1 Rui Matos 2017-04-24 12:52:19 UTC
Created attachment 350290 [details] [review]
atk-adaptor/bridge: Fix GList handling resulting in memory corruption

As pointed out by this valgrind log:

==2809== Thread 1:
==2809== Invalid write of size 8
==2809==    at 0x18FCF001: remove_events (bridge.c:759)
==2809==    by 0x18FCF001: handle_event_listener_deregistered (bridge.c:788)
==2809==    by 0x18FCF001: signal_filter (bridge.c:827)
==2809==    by 0x200ECDFD: dbus_connection_dispatch (dbus-connection.c:4631)
==2809==    by 0x1FEBD0F4: ??? (in /usr/lib64/libatspi.so.0.0.1)
==2809==    by 0xFD8D4C8: g_main_dispatch (gmain.c:3201)
==2809==    by 0xFD8D4C8: g_main_context_dispatch (gmain.c:3854)
==2809==    by 0xFD8D817: g_main_context_iterate.isra.21 (gmain.c:3927)
==2809==    by 0xFD8DAE9: g_main_loop_run (gmain.c:4123)
==2809==    by 0xDFF84B4: gtk_main (in /usr/lib64/libgtk-3.so.0.2200.10)
==2809==    by 0x403DE0: main (in /usr/bin/evolution)
==2809==  Address 0x29f22540 is 16 bytes inside a block of size 24 free'd
==2809==    at 0x4C2ACDD: free (vg_replace_malloc.c:530)
==2809==    by 0xFD92BCD: g_free (gmem.c:189)
==2809==    by 0xFDAA518: g_slice_free1 (gslice.c:1136)
==2809==    by 0xFD89463: g_list_remove (glist.c:521)
==2809==    by 0x18FCF000: remove_events (bridge.c:759)
==2809==    by 0x18FCF000: handle_event_listener_deregistered (bridge.c:788)
==2809==    by 0x18FCF000: signal_filter (bridge.c:827)
==2809==    by 0x200ECDFD: dbus_connection_dispatch (dbus-connection.c:4631)
==2809==    by 0x1FEBD0F4: ??? (in /usr/lib64/libatspi.so.0.0.1)
==2809==    by 0xFD8D4C8: g_main_dispatch (gmain.c:3201)
==2809==    by 0xFD8D4C8: g_main_context_dispatch (gmain.c:3854)
==2809==    by 0xFD8D817: g_main_context_iterate.isra.21 (gmain.c:3927)
==2809==    by 0xFD8DAE9: g_main_loop_run (gmain.c:4123)
==2809==    by 0xDFF84B4: gtk_main (in /usr/lib64/libgtk-3.so.0.2200.10)
==2809==    by 0x403DE0: main (in /usr/bin/evolution)
==2809==  Block was alloc'd at
==2809==    at 0x4C29BE3: malloc (vg_replace_malloc.c:299)
==2809==    by 0xFD92ABD: g_malloc (gmem.c:94)
==2809==    by 0xFDA9EFD: g_slice_alloc (gslice.c:1025)
==2809==    by 0xFD89983: g_list_append (glist.c:261)
==2809==    by 0x18FCE7EE: add_event (bridge.c:80)
==2809==    by 0x18FCE7EE: add_event_from_iter (bridge.c:217)
==2809==    by 0x18FCEEF6: handle_event_listener_registered (bridge.c:721)
==2809==    by 0x18FCEEF6: signal_filter (bridge.c:825)
==2809==    by 0x200ECDFD: dbus_connection_dispatch (dbus-connection.c:4631)
==2809==    by 0x1FEBD0F4: ??? (in /usr/lib64/libatspi.so.0.0.1)
==2809==    by 0xFD8D4C8: g_main_dispatch (gmain.c:3201)
==2809==    by 0xFD8D4C8: g_main_context_dispatch (gmain.c:3854)
==2809==    by 0xFD8D817: g_main_context_iterate.isra.21 (gmain.c:3927)
==2809==    by 0xFD8DAE9: g_main_loop_run (gmain.c:4123)
==2809==    by 0xDFF84B4: gtk_main (in /usr/lib64/libgtk-3.so.0.2200.10)

This line:

list->prev = g_list_remove (list->prev, evdata);

writes over free'd memory since the list link pointed to by the 'list'
pointer is free'd by g_list_remove(). We can use g_list_delete_link()
instead to achieve the intended result (and not re-iterate the whole
list) with less code overall.

Thanks to Milan Crha <mcrha@redhat.com> for investigating and
providing the valgring log.
Comment 2 Rui Matos 2017-04-25 15:57:37 UTC
Comment on attachment 350290 [details] [review]
atk-adaptor/bridge: Fix GList handling resulting in memory corruption

This was pushed, closing.

https://git.gnome.org/browse/at-spi2-atk/commit/?id=7cdc1f91c9802b0b8ecd2afea38c1717b1921736