From 6d734b895a89462f7226ddb342ae02e2b49efc69 Mon Sep 17 00:00:00 2001 From: Ting-Wei Lan Date: Sun, 17 Nov 2019 22:39:07 +0800 Subject: [PATCH] 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 | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gio/kqueue/gkqueuefilemonitor.c b/gio/kqueue/gkqueuefilemonitor.c index fd0db4e29..c6e4ec4b8 100644 --- a/gio/kqueue/gkqueuefilemonitor.c +++ b/gio/kqueue/gkqueuefilemonitor.c @@ -122,7 +122,7 @@ static gboolean g_kqueue_file_monitor_is_supported (void); static kqueue_sub *_kqsub_new (gchar *, gchar *, GKqueueFileMonitor *, GFileMonitorSource *); static void _kqsub_free (kqueue_sub *); -static gboolean _kqsub_cancel (kqueue_sub *); +static void _kqsub_cancel (kqueue_sub *); #ifndef O_EVTONLY @@ -547,7 +547,7 @@ _kqsub_free (kqueue_sub *sub) g_slice_free (kqueue_sub, sub); } -static gboolean +static void _kqsub_cancel (kqueue_sub *sub) { /* WARNING: Before calling this function, you must hold a lock on kq_lock @@ -563,7 +563,6 @@ _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; } close (sub->fd); sub->fd = -1; @@ -576,8 +575,6 @@ _kqsub_cancel (kqueue_sub *sub) dl_free (sub->deps); sub->deps = NULL; } - - return TRUE; } gboolean