Bug#862219: unblock: at-spi2-atk/2.22.0-2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
Hello,
Upstream of at-spi has released some serious fixes for at-spi2-atk,
which I have uploaded as at-spi2-atk 2.22.0-2, and attached to this
mail.
git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736 fixes a memory corruption
reported by valgrind, which could make basically any application crash
when the Orca screen reader is running, when processing events. It does
so by just using the right glib function for what the buggy code meant
to do.
git-8d3cc68f7bc62c7015d986212be0d5d776920ee2 fixes memory references
after dropping a refcount from the object (thus potentially freed), also
leading to potential crash of any application when the Orca screen
reader is running.
unblock at-spi2-atk/2.22.0-2
-- System Information:
Debian Release: 9.0
APT prefers testing
APT policy: (990, 'testing'), (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'buildd-unstable'), (500, 'unstable'), (500, 'stable'), (500, 'oldstable'), (1, 'experimental-debug'), (1, 'buildd-experimental'), (1, 'experimental')
Architecture: amd64
(x86_64)
Foreign Architectures: i386
Kernel: Linux 4.11.0 (SMP w/4 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
--
Samuel
if (argc > 1 && strcmp(argv[1], "-advice") == 0) {
printf("Don't Panic!\n");
exit(42);
}
-- Arnold Robbins in the LJ of February '95, describing RCS
diff -Nru at-spi2-atk-2.22.0/debian/changelog at-spi2-atk-2.22.0/debian/changelog
--- at-spi2-atk-2.22.0/debian/changelog 2016-10-01 22:09:42.000000000 +0200
+++ at-spi2-atk-2.22.0/debian/changelog 2017-05-09 21:35:33.000000000 +0200
@@ -1,3 +1,12 @@
+at-spi2-atk (2.22.0-2) unstable; urgency=medium
+
+ * patches/git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736: Fix GList handling
+ resulting in memory corruption.
+ * patches/git-8d3cc68f7bc62c7015d986212be0d5d776920ee2: Fix use after free
+ when returned objects hold only one ref.
+
+ -- Samuel Thibault <sthibault@debian.org> Tue, 09 May 2017 21:35:33 +0200
+
at-spi2-atk (2.22.0-1) unstable; urgency=medium
* New upstream release.
diff -Nru at-spi2-atk-2.22.0/debian/patches/git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736 at-spi2-atk-2.22.0/debian/patches/git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736
--- at-spi2-atk-2.22.0/debian/patches/git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736 1970-01-01 01:00:00.000000000 +0100
+++ at-spi2-atk-2.22.0/debian/patches/git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736 2017-05-09 21:35:33.000000000 +0200
@@ -0,0 +1,101 @@
+commit 7cdc1f91c9802b0b8ecd2afea38c1717b1921736
+Author: Rui Matos <tiagomatos@gmail.com>
+Date: Mon Apr 24 14:39:05 2017 +0200
+
+ 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.
+
+ https://bugzilla.gnome.org/show_bug.cgi?id=781658
+
+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
+ {
diff -Nru at-spi2-atk-2.22.0/debian/patches/git-8d3cc68f7bc62c7015d986212be0d5d776920ee2 at-spi2-atk-2.22.0/debian/patches/git-8d3cc68f7bc62c7015d986212be0d5d776920ee2
--- at-spi2-atk-2.22.0/debian/patches/git-8d3cc68f7bc62c7015d986212be0d5d776920ee2 1970-01-01 01:00:00.000000000 +0100
+++ at-spi2-atk-2.22.0/debian/patches/git-8d3cc68f7bc62c7015d986212be0d5d776920ee2 2017-05-09 21:35:33.000000000 +0200
@@ -0,0 +1,85 @@
+commit 8d3cc68f7bc62c7015d986212be0d5d776920ee2
+Author: Milan Crha <mcrha@redhat.com>
+Date: Mon May 8 17:21:58 2017 -0500
+
+ Fix use after free when returned objects hold only one ref
+
+ It seems that not all code expects atk_object_ref_accessible_child()
+ returning NULL, neither that it can return an object with only one
+ reference, thus the following unref in the code can cause use-after-free
+ eventually.
+
+ At least the chunk in impl_GetChildAtIndex() avoids runtime warning about
+ invalid object being passed to g_object_unref(), which happened, in this
+ case, when evolution returned NULL. Evolution returns objects with one
+ reference only often, which tries to address the other chunks here.
+
+ https://bugzilla.gnome.org/show_bug.cgi?id=781716
+
+diff --git a/atk-adaptor/adaptors/accessible-adaptor.c b/atk-adaptor/adaptors/accessible-adaptor.c
+index 058b116..572e4f8 100644
+--- a/atk-adaptor/adaptors/accessible-adaptor.c
++++ b/atk-adaptor/adaptors/accessible-adaptor.c
+@@ -182,7 +182,8 @@ impl_GetChildAtIndex (DBusConnection * bus,
+ }
+ child = atk_object_ref_accessible_child (object, i);
+ reply = spi_object_return_reference (message, child);
+- g_object_unref (child);
++ if (child)
++ g_object_unref (child);
+
+ return reply;
+ }
+diff --git a/atk-adaptor/adaptors/collection-adaptor.c b/atk-adaptor/adaptors/collection-adaptor.c
+index 42ea073..b57c5f6 100644
+--- a/atk-adaptor/adaptors/collection-adaptor.c
++++ b/atk-adaptor/adaptors/collection-adaptor.c
+@@ -494,9 +494,12 @@ sort_order_canonical (MatchRulePrivate * mrp, GList * ls,
+ {
+ AtkObject *child = atk_object_ref_accessible_child (obj, i);
+
+- g_object_unref (child);
++ if (!child)
++ continue;
++
+ if (prev && child == pobj)
+ {
++ g_object_unref (child);
+ return kount;
+ }
+
+@@ -517,6 +520,7 @@ sort_order_canonical (MatchRulePrivate * mrp, GList * ls,
+ kount = sort_order_canonical (mrp, ls, kount,
+ max, child, 0, TRUE,
+ pobj, recurse, traverse);
++ g_object_unref (child);
+ }
+ return kount;
+ }
+@@ -559,19 +563,23 @@ sort_order_rev_canonical (MatchRulePrivate * mrp, GList * ls,
+ and get it's last descendant.
+ First, get the previous sibling */
+ nextobj = atk_object_ref_accessible_child (parent, indexinparent - 1);
+- g_object_unref (nextobj);
+
+ /* Now, drill down the right side to the last descendant */
+- while (atk_object_get_n_accessible_children (nextobj) > 0)
++ while (nextobj && atk_object_get_n_accessible_children (nextobj) > 0)
+ {
+- nextobj = atk_object_ref_accessible_child (nextobj,
++ AtkObject *follow;
++
++ follow = atk_object_ref_accessible_child (nextobj,
+ atk_object_get_n_accessible_children
+ (nextobj) - 1);
+ g_object_unref (nextobj);
++ nextobj = follow;
+ }
+ /* recurse with the last descendant */
+ kount = sort_order_rev_canonical (mrp, ls, kount, max,
+ nextobj, TRUE, pobj);
++ if (nextobj)
++ g_object_unref (nextobj);
+ }
+ else if (max == 0 || kount < max)
+ {
diff -Nru at-spi2-atk-2.22.0/debian/patches/series at-spi2-atk-2.22.0/debian/patches/series
--- at-spi2-atk-2.22.0/debian/patches/series 2016-10-01 21:45:09.000000000 +0200
+++ at-spi2-atk-2.22.0/debian/patches/series 2017-05-09 21:35:33.000000000 +0200
@@ -1 +1,3 @@
main_context
+git-7cdc1f91c9802b0b8ecd2afea38c1717b1921736
+git-8d3cc68f7bc62c7015d986212be0d5d776920ee2
Reply to: