[glib-networking/mcatanzaro/#97: 4/4] gnutls: Fix crash when handshake_context is reset too late



commit 630d26e333ab316ca04b941d6bb4f861df13d52b
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Thu Oct 31 14:29:15 2019 -0500

    gnutls: Fix crash when handshake_context is reset too late
    
    If g_task_return_* returns immediately, via g_task_return_now(), then
    application code could start using the GTlsConnection again inside the
    call to g_task_return_now(), before handshake_thread_completed() has
    finished. That means a new handshake operation could get started before
    the previous handshake_context is cleared. It happens in practice when
    the server tries to rehandshake.
    
    See #97, though this only fixes one instance of #97. There is another
    mystery condition where handshake_context fails to get unset that I have
    not yet found, so this issue is not completely solved.

 tls/base/gtlsconnection-base.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 2d092e4..5f7cb32 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1685,20 +1685,33 @@ async_handshake_thread_completed (GObject      *object,
     need_finish_handshake = FALSE;
   g_mutex_unlock (&priv->op_mutex);
 
+  /* We have to clear handshake_context before g_task_return_* because it can
+   * return immediately to application code inside g_task_return_*,
+   * and the application code could then start a new TLS operation.
+   *
+   * But we can't clear until after finish_handshake().
+   */
   if (need_finish_handshake)
     {
       success = finish_handshake (tls, G_TASK (result), &error);
+
+      g_clear_pointer (&priv->handshake_context, g_main_context_unref);
+
       if (success)
         g_task_return_boolean (caller_task, TRUE);
       else
         g_task_return_error (caller_task, error);
     }
-  else if (priv->handshake_error)
-    g_task_return_error (caller_task, g_error_copy (priv->handshake_error));
   else
-    g_task_return_boolean (caller_task, TRUE);
+    {
+      g_clear_pointer (&priv->handshake_context, g_main_context_unref);
+
+      if (priv->handshake_error)
+        g_task_return_error (caller_task, g_error_copy (priv->handshake_error));
+      else
+        g_task_return_boolean (caller_task, TRUE);
+    }
 
-  g_clear_pointer (&priv->handshake_context, g_main_context_unref);
   g_object_unref (caller_task);
 }
 


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