From 22656f16c29591207c667362e2a42fd348fe8494 Mon Sep 17 00:00:00 2001

From: Martin Pieuchot <mpi@openbsd.org>

Date: Fri, 28 Apr 2017 15:06:52 +0200

Subject: [PATCH] kqueue: fix use-after-free of ``kqueue_sub''.

Since ``kqueue_sub'' are not refcounted it is common to see a thread

freeing one of them while another thread is manipulating them.  This

leads to crashs reported in:

	https://bugzilla.gnome.org/show_bug.cgi?id=739424

To prevent such crash, make sure the threads are holding ``hash_lock''

when manipulating such items.

---

 gio/kqueue/kqueue-helper.c | 6 ++++--

 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gio/kqueue/kqueue-helper.c b/gio/kqueue/kqueue-helper.c

index d4e66cd4d..84b9ef164 100644

--- gio/kqueue/kqueue-helper.c

+++ gio/kqueue/kqueue-helper.c

@@ -291,10 +291,10 @@ process_kqueue_notifications (GIOChannel   *gioc,

   G_LOCK (hash_lock);

   sub = (kqueue_sub *) g_hash_table_lookup (subs_hash_table, GINT_TO_POINTER (n.fd));

-  G_UNLOCK (hash_lock);

   if (sub == NULL)

     {

+      G_UNLOCK (hash_lock);

       KH_W ("Got a notification for a deleted or non-existing subscription %d",

              n.fd);

       return TRUE;

@@ -336,6 +336,7 @@ process_kqueue_notifications (GIOChannel   *gioc,

         g_file_monitor_source_handle_event (source, mask, NULL, NULL, NULL, g_get_monotonic_time ());

     }

+  G_UNLOCK (hash_lock);

   return TRUE;

 }

@@ -451,13 +452,14 @@ _kh_start_watching (kqueue_sub *sub)

   G_LOCK (hash_lock);

   g_hash_table_insert (subs_hash_table, GINT_TO_POINTER (sub->fd), sub);

-  G_UNLOCK (hash_lock);

   _kqueue_thread_push_fd (sub->fd);

   /* Bump the kqueue thread. It will pick up a new sub entry to monitor */

   if (!_ku_write (kqueue_socket_pair[0], "A", 1))

     KH_W ("Failed to bump the kqueue thread (add fd, error %d)", errno);

+  G_UNLOCK (hash_lock);

+

   return TRUE;

 }

-- 

2.12.2

From e305fe971e4647d971428a772b7290b9c308a96f Mon Sep 17 00:00:00 2001

From: Steven McDonald <steven@steven-mcdonald.id.au>

Date: Sun, 12 Feb 2017 11:02:55 +1100

Subject: gio: Always purge kqueue subs from missing list

Previously, _kh_cancel_sub assumed that it only needed to call

_km_remove if sub did not exist in subs_hash_table. This is erroneous

because the complementary operation, _km_add_missing, can be called

from process_kqueue_notifications, in which context sub can *only* have

come from subs_hash_table.

Since _km_remove is implemented using g_slist_remove, which is

documented to be a noop if the list does not contain the element to be

removed, it is safe to call _km_remove unconditionally here.

https://bugzilla.gnome.org/show_bug.cgi?id=778515

---

 gio/kqueue/kqueue-helper.c | 15 +++++----------

 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/gio/kqueue/kqueue-helper.c b/gio/kqueue/kqueue-helper.c

index 4671396..d4e66cd 100644

--- gio/kqueue/kqueue-helper.c

+++ gio/kqueue/kqueue-helper.c

@@ -498,22 +498,17 @@ _kh_add_sub (kqueue_sub *sub)

 gboolean

 _kh_cancel_sub (kqueue_sub *sub)

 {

-  gboolean missing = FALSE;

+  gboolean removed = FALSE;

   g_assert (kqueue_socket_pair[0] != -1);

   g_assert (sub != NULL);

+  _km_remove (sub);

+

   G_LOCK (hash_lock);

-  missing = !g_hash_table_remove (subs_hash_table, GINT_TO_POINTER (sub->fd));

+  removed = g_hash_table_remove (subs_hash_table, GINT_TO_POINTER (sub->fd));

   G_UNLOCK (hash_lock);

-  if (missing)

-    {

-      /* If there were no fd for this subscription, file is still

-       * missing. */

-      KH_W ("Removing subscription from missing");

-      _km_remove (sub);

-    }

-  else

+  if (removed)

     {

       /* fd will be closed in the kqueue thread */

       _kqueue_thread_remove_fd (sub->fd);

-- 

cgit v0.12