From 695ee1023547063b97141e4220e9224ca4cb279e Mon Sep 17 00:00:00 2001
From: Philip Withnall <withnall@endlessm.com>
Date: Wed, 29 Jan 2020 17:37:12 +0000
Subject: [PATCH] malcontent-control: Fix use-after-free when closing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Sometimes, when closing the application,
`flush_update_blacklisted_apps()` would be called after
`MctRestrictApplicationsSelector` had been destroyed, leading to a
critical.

This was because the `MctRestrictApplicationsDialog` was being disposed
early due to its `destroy-with-parent` property being set. The dispose
function of `MctUserControls` was run several times due to GTK calling
`g_object_run_dispose()`, and the critical would be emitted the second
time.

Make the dispose function’s call to `flush_update_blacklisted_apps()` be
safe for multiple dispose calls, and ensure the dialog isn’t destroyed
too early.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
---
 malcontent-control/user-controls.c  | 12 +++++++++++-
 malcontent-control/user-controls.ui |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/malcontent-control/user-controls.c b/malcontent-control/user-controls.c
index 7a10f01..ee1f8ca 100644
--- a/malcontent-control/user-controls.c
+++ b/malcontent-control/user-controls.c
@@ -62,6 +62,7 @@ struct _MctUserControls
   guint         selected_age; /* @oars_disabled_age to disable OARS */
 
   guint         blacklist_apps_source_id;
+  gboolean      flushed_on_dispose;
 };
 
 static gboolean blacklist_apps_cb (gpointer data);
@@ -650,6 +651,9 @@ mct_user_controls_finalize (GObject *object)
   g_clear_pointer (&self->filter, mct_app_filter_unref);
   g_clear_object (&self->manager);
 
+  /* Hopefully we don’t have data loss. */
+  g_assert (self->flushed_on_dispose);
+
   G_OBJECT_CLASS (mct_user_controls_parent_class)->finalize (object);
 }
 
@@ -659,7 +663,13 @@ mct_user_controls_dispose (GObject *object)
 {
   MctUserControls *self = (MctUserControls *)object;
 
-  flush_update_blacklisted_apps (self);
+  /* Since GTK calls g_object_run_dispose(), dispose() may be called multiple
+   * times. We definitely want to save any unsaved changes, but don’t need to
+   * do it multiple times, and after the first g_object_run_dispose() call,
+   * none of our child widgets are still around to extract data from anyway. */
+  if (!self->flushed_on_dispose)
+    flush_update_blacklisted_apps (self);
+  self->flushed_on_dispose = TRUE;
 
   G_OBJECT_CLASS (mct_user_controls_parent_class)->dispose (object);
 }
diff --git a/malcontent-control/user-controls.ui b/malcontent-control/user-controls.ui
index 2439051..4db1404 100644
--- a/malcontent-control/user-controls.ui
+++ b/malcontent-control/user-controls.ui
@@ -527,7 +527,7 @@
   <object class="MctRestrictApplicationsDialog" id="restrict_applications_dialog">
     <property name="visible">False</property>
     <property name="modal">True</property>
-    <property name="destroy-with-parent">True</property>
+    <property name="destroy-with-parent">False</property>
     <property name="use-header-bar">1</property>
     <signal name="delete-event" handler="on_restrict_applications_dialog_delete_event_cb" />
     <signal name="response" handler="on_restrict_applications_dialog_response_cb" />