[evolution-ews] Bug #699558 - Use-after-free of a cancellable during GAL update

[Milan Crha <mcrha src gnome org>]

commit c51b994387b9116887faad95180871e935756d7a
Author: Milan Crha <mcrha redhat com>
Date:   Thu Aug 15 14:36:04 2013 +0200

    Bug #699558 - Use-after-free of a cancellable during GAL update

 src/addressbook/e-book-backend-ews.c |   47 ++++++++++++++++++++++++---------
 src/server/e-ews-connection.c        |   14 +++++-----
 src/server/e-ews-connection.h        |    3 ++
 3 files changed, 44 insertions(+), 20 deletions(-)
---
diff --git a/src/addressbook/e-book-backend-ews.c b/src/addressbook/e-book-backend-ews.c
index ecaebd4..ca54ee9 100644
--- a/src/addressbook/e-book-backend-ews.c
+++ b/src/addressbook/e-book-backend-ews.c
@@ -2386,9 +2386,6 @@ ews_gal_store_contact (EContact *contact,
                data->contact_collector = NULL;
                data->collected_length = 0;
        }
-
-       if (percent == 100)
-               e_book_backend_notify_complete (E_BOOK_BACKEND (data->cbews));
 }
 
 static gint det_sort_func (gconstpointer _a, gconstpointer _b)
@@ -2436,6 +2433,11 @@ ews_replace_gal_in_db (EBookBackendEws *cbews,
        data.cbews = cbews;
 
        ret = ews_oab_decoder_decode (eod, ews_gal_store_contact, &data, cancellable, error);
+
+       /* always notify views as complete, to not left anything behind,
+          if the decode was cancelled before full completion */
+       e_book_backend_notify_complete (E_BOOK_BACKEND (cbews));
+
        if (!ret)
               return ret;
 
@@ -2464,11 +2466,14 @@ ebews_start_gal_sync (gpointer data)
        guint32 old_seq = 0;
        guint32 delta_size = 0;
        CamelEwsSettings *ews_settings;
+       GCancellable *cancellable;
 
        cbews = (EBookBackendEws *) data;
        ews_settings = book_backend_ews_get_collection_settings (cbews);
        priv = cbews->priv;
 
+       cancellable = g_object_ref (priv->cancellable);
+
        oab_cnc = e_ews_connection_new (priv->oab_url, ews_settings);
 
        password = e_ews_connection_dup_password (priv->cnc);
@@ -2493,7 +2498,7 @@ ebews_start_gal_sync (gpointer data)
 
        if (!e_ews_connection_get_oal_detail_sync (
                oab_cnc, priv->folder_id, NULL, old_etag, &full_l, &etag,
-               priv->cancellable, &error)) {
+               cancellable, &error)) {
                ret = FALSE;
                goto exit;
        }
@@ -2533,7 +2538,7 @@ ebews_start_gal_sync (gpointer data)
                deltas = NULL;
        }
 
-       uncompressed_filename = ews_download_gal (cbews, full, deltas, old_seq, priv->cancellable, &error);
+       uncompressed_filename = ews_download_gal (cbews, full, deltas, old_seq, cancellable, &error);
        if (!uncompressed_filename) {
                ret = FALSE;
                goto exit;
@@ -2547,7 +2552,7 @@ ebews_start_gal_sync (gpointer data)
        }
 
        d (printf ("Ewsgal: Replacing old gal with new gal contents in db \n");)
-       ret = ews_replace_gal_in_db (cbews, uncompressed_filename, priv->cancellable, &error);
+       ret = ews_replace_gal_in_db (cbews, uncompressed_filename, cancellable, &error);
        if (!ret)
                goto exit;
 
@@ -2572,6 +2577,8 @@ ebews_start_gal_sync (gpointer data)
        d (printf ("Ews gal: sync successful complete \n");)
 
 exit:
+       g_clear_object (&cancellable);
+
        if (error) {
                g_warning ("Unable to update gal : %s \n", error->message);
                g_clear_error (&error);
@@ -2970,7 +2977,7 @@ ebews_fetch_items (EBookBackendEws *ebews,
                d_name = e_ews_item_get_subject (item);
                e_ews_connection_expand_dl_sync (
                        cnc, EWS_PRIORITY_MEDIUM, mb, &members,
-                       &includes_last, priv->cancellable, error);
+                       &includes_last, cancellable, error);
                if (*error)
                        goto cleanup;
 
@@ -3026,6 +3033,7 @@ ebews_start_sync (gpointer data)
        GList *list, *link;
        gchar *sync_state, *status_message = NULL;
        gboolean includes_last_item;
+       GCancellable *cancellable;
        GError *error = NULL;
 
        ebews = (EBookBackendEws *) data;
@@ -3037,6 +3045,8 @@ ebews_start_sync (gpointer data)
        if (!priv->cnc)
                return TRUE;
 
+       cancellable = g_object_ref (priv->cancellable);
+
        status_message = g_strdup (_("Syncing contacts..."));
        list = e_book_backend_list_views (E_BOOK_BACKEND (ebews));
        for (link = list; link != NULL; link = g_list_next (link))
@@ -3060,7 +3070,7 @@ ebews_start_sync (gpointer data)
                        &sync_state,
                        &includes_last_item,
                        &items_created, &items_updated,
-                       &items_deleted, priv->cancellable, &error);
+                       &items_deleted, cancellable, &error);
 
                g_free (old_sync_state);
 
@@ -3071,7 +3081,7 @@ ebews_start_sync (gpointer data)
 
                        e_ews_connection_sync_folder_items_sync (priv->cnc, EWS_PRIORITY_MEDIUM, NULL, 
priv->folder_id, "IdOnly", NULL, EWS_MAX_FETCH_COUNT,
                                &sync_state, &includes_last_item, &items_created, &items_updated, 
&items_deleted,
-                               priv->cancellable, &error);
+                               cancellable, &error);
                }
 
                if (error)
@@ -3081,7 +3091,7 @@ ebews_start_sync (gpointer data)
                        ebews_sync_deleted_items (ebews, items_deleted, &error);
 
                if (items_created)
-                       ebews_fetch_items (ebews, items_created, TRUE, NULL, priv->cancellable, &error);
+                       ebews_fetch_items (ebews, items_created, TRUE, NULL, cancellable, &error);
 
                if (error) {
                        g_slist_free_full (items_updated, g_object_unref);
@@ -3090,7 +3100,7 @@ ebews_start_sync (gpointer data)
                }
 
                if (items_updated)
-                       ebews_fetch_items (ebews, items_updated, TRUE, NULL, priv->cancellable, &error);
+                       ebews_fetch_items (ebews, items_updated, TRUE, NULL, cancellable, &error);
 
                if (error)
                        break;
@@ -3109,6 +3119,8 @@ ebews_start_sync (gpointer data)
                e_data_book_view_notify_progress (E_DATA_BOOK_VIEW (link->data), -1, NULL);
        g_list_free_full (list, g_object_unref);
 
+       g_clear_object (&cancellable);
+
        if (error) {
                g_warning ("Error Syncing Contacts: Folder %s Error: %s", priv->folder_id, error->message);
                g_clear_error (&error);
@@ -3125,6 +3137,10 @@ delta_thread (gpointer data)
        EBookBackendEwsPrivate *priv = ebews->priv;
        gint64 end_time;
 
+       g_mutex_lock (&priv->dlock->mutex);
+       g_object_ref (ebews);
+       g_mutex_unlock (&priv->dlock->mutex);
+
        while (TRUE)    {
                gboolean succeeded;
 
@@ -3135,6 +3151,10 @@ delta_thread (gpointer data)
 
                g_mutex_lock (&priv->dlock->mutex);
 
+               /* in case this is the last reference, then this cannot join
+                  the itself thread in dispose */
+               e_ews_connection_utils_unref_in_thread (ebews);
+
                if (!succeeded || priv->dlock->exit)
                        break;
 
@@ -3144,6 +3164,8 @@ delta_thread (gpointer data)
                if (priv->dlock->exit)
                        break;
 
+               g_object_ref (ebews);
+
                g_mutex_unlock (&priv->dlock->mutex);
        }
 
@@ -3682,9 +3704,8 @@ e_book_backend_ews_dispose (GObject *object)
        if (priv->dlock) {
                g_mutex_lock (&priv->dlock->mutex);
                priv->dlock->exit = TRUE;
-               g_mutex_unlock (&priv->dlock->mutex);
-
                g_cond_signal (&priv->dlock->cond);
+               g_mutex_unlock (&priv->dlock->mutex);
 
                if (priv->dthread)
                        g_thread_join (priv->dthread);
diff --git a/src/server/e-ews-connection.c b/src/server/e-ews-connection.c
index 1287ee5..ad9f1e4 100644
--- a/src/server/e-ews-connection.c
+++ b/src/server/e-ews-connection.c
@@ -189,8 +189,8 @@ ews_unref_in_thread_func (gpointer data)
        return NULL;
 }
 
-static void
-ews_unref_in_thread (gpointer object)
+void
+e_ews_connection_utils_unref_in_thread (gpointer object)
 {
        GThread *thread;
 
@@ -312,7 +312,7 @@ ews_connection_scheduled_cb (gpointer user_data)
        if (sd->message)
                g_object_unref (sd->message);
        /* in case this is the last reference */
-       ews_unref_in_thread (sd->cnc);
+       e_ews_connection_utils_unref_in_thread (sd->cnc);
        g_free (sd);
 
        return FALSE;
@@ -513,7 +513,7 @@ ews_active_job_done (EEwsConnection *cnc,
        /* the 'simple' holds reference on 'cnc' and this function
         * is called in a dedicated thread, which 'cnc' joins on dispose,
         * thus to avoid race condition, unref the object in its own thread */
-       ews_unref_in_thread (ews_node->simple);
+       e_ews_connection_utils_unref_in_thread (ews_node->simple);
        g_free (ews_node);
 }
 
@@ -2220,7 +2220,7 @@ autodiscover_response_cb (SoupSession *session,
         * in connection's dispose, trying to wait on the end of itself, thus it's
         * safer to unref the 'simple' in a dedicated thread.
        */
-       ews_unref_in_thread (simple);
+       e_ews_connection_utils_unref_in_thread (simple);
 }
 
 static void post_restarted (SoupMessage *msg, gpointer data)
@@ -2731,7 +2731,7 @@ oal_response_cb (SoupSession *soup_session,
         * for cases when the complete_in_idle is finished before the unref call, when
         * the cnc will be left with the last reference and thus cannot join the soup_thread
         * while still in it, the unref is done in a dedicated thread. */
-       ews_unref_in_thread (simple);
+       e_ews_connection_utils_unref_in_thread (simple);
 }
 
 static void
@@ -3005,7 +3005,7 @@ oal_download_response_cb (SoupSession *soup_session,
        }
 
        g_simple_async_result_complete_in_idle (simple);
-       ews_unref_in_thread (simple);
+       e_ews_connection_utils_unref_in_thread (simple);
 }
 
 static void
diff --git a/src/server/e-ews-connection.h b/src/server/e-ews-connection.h
index 2e13dc7..53d3fc7 100644
--- a/src/server/e-ews-connection.h
+++ b/src/server/e-ews-connection.h
@@ -208,6 +208,9 @@ typedef struct {
 void           ews_oal_free                    (EwsOAL *oal);
 void           ews_oal_details_free            (EwsOALDetails *details);
 
+void           e_ews_connection_utils_unref_in_thread
+                                               (gpointer object);
+
 GType          e_ews_connection_get_type       (void);
 EEwsConnection *e_ews_connection_new           (const gchar *uri,
                                                 CamelEwsSettings *settings);