[glib/wip/lantw/kqueue-Do-not-return-early-from-_kqsub_cancel] kqueue: Do not return early from _kqsub_cancel



commit 89b9a10f7f574bc3a6c27e42f22013a3aff18667
Author: Ting-Wei Lan <lantw src gnome org>
Date:   Sun Nov 17 22:39:07 2019 +0800

    kqueue: Do not return early from _kqsub_cancel
    
    _kqsub_free assumes the caller has called _kqsub_cancel before calling
    it. It checks if both 'deps' and 'fd' have been freed and aborts when
    the condition is not met. Since the only caller of _kqsub_free is
    g_kqueue_file_monitor_finalize, which does call _kqsub_cancel before
    calling _kqsub_free, it seems to be correct for _kqsub_free to assert
    values of these two members there.
    
    However, it is possible for _kqsub_cancel to return early without
    freeing any resource _kqsub_free expects to be freed. When the kevent
    call fails, _kqsub_cancel does not free anything and _kqsub_free aborts
    with assertion failure. This is an unexpected behavior, and it can be
    fixed by always freeing resources in _kqsub_cancel.
    
    Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1935

 gio/kqueue/gkqueuefilemonitor.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
---
diff --git a/gio/kqueue/gkqueuefilemonitor.c b/gio/kqueue/gkqueuefilemonitor.c
index fd0db4e29..0c9a0f63f 100644
--- a/gio/kqueue/gkqueuefilemonitor.c
+++ b/gio/kqueue/gkqueuefilemonitor.c
@@ -553,6 +553,7 @@ _kqsub_cancel (kqueue_sub *sub)
   /* WARNING: Before calling this function, you must hold a lock on kq_lock
    * or you will cause use-after-free in g_kqueue_file_monitor_callback. */
 
+  gboolean ok = TRUE;
   struct kevent ev;
 
   /* Remove the event and close the file descriptor to automatically
@@ -563,7 +564,7 @@ _kqsub_cancel (kqueue_sub *sub)
       if (kevent (kq_queue, &ev, 1, NULL, 0, NULL) == -1)
         {
           g_warning ("Unable to remove event for %s: %s", sub->filename, g_strerror (errno));
-          return FALSE;
+          ok = FALSE;
         }
       close (sub->fd);
       sub->fd = -1;
@@ -577,7 +578,7 @@ _kqsub_cancel (kqueue_sub *sub)
       sub->deps = NULL;
     }
 
-  return TRUE;
+  return ok;
 }
 
 gboolean


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