[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#862219: marked as done (unblock: at-spi2-atk/2.22.0-2)



Your message dated Wed, 17 May 2017 05:25:00 +0000
with message-id <94cd9eec-7de1-5b94-26ac-febb140dde71@thykier.net>
and subject line Re: Bug#862219: unblock: at-spi2-atk/2.22.0-2
has caused the Debian Bug report #862219,
regarding unblock: at-spi2-atk/2.22.0-2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
862219: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=862219
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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

--- End Message ---
--- Begin Message ---
Cyril Brulebois:
> Niels Thykier <niels@thykier.net> (2017-05-12):
>>> 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
>>>
>>> [...]
>>
>> Ack from here, CC'ing KiBi for a d-i ack.
> 
> No objections, thanks.
> 
> 
> KiBi.
> 

Unblocked, thanks.

~Niels

--- End Message ---

Reply to: