From 113d9c15ac9d1cc5f120d859d284ffb55f80d54e Mon Sep 17 00:00:00 2001 From: Rui Matos Date: Mon, 24 Apr 2017 14:39:05 +0200 Subject: [PATCH] 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 for investigating and providing the valgring log. https://bugzilla.gnome.org/show_bug.cgi?id=781658 --- atk-adaptor/bridge.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/atk-adaptor/bridge.c b/atk-adaptor/bridge.c index 7de84d4..0b2b736 100644 --- a/atk-adaptor/bridge.c +++ b/atk-adaptor/bridge.c @@ -748,22 +748,17 @@ remove_events (const char *bus_name, const char *event) if (!g_strcmp0 (evdata->bus_name, bus_name) && spi_event_is_subtype (evdata->data, remove_data)) { + GList *next; GList *events = spi_global_app_data->events; + g_strfreev (evdata->data); g_free (evdata->bus_name); g_slist_free_full (evdata->properties, free_property_definition); g_free (evdata); - if (list->prev) - { - GList *next = list->next; - list->prev = g_list_remove (list->prev, evdata); - list = next; - } - else - { - spi_global_app_data->events = g_list_remove (events, evdata); - list = spi_global_app_data->events; - } + + next = list->next; + spi_global_app_data->events = g_list_delete_link (events, list); + list = next; } else { -- 2.9.3