Fix crash on SKE failure.

Adds checks to prevent SKE failure notification callbacks from being called
multiple times for the same SKE instance.  This would often happen, for
example, if we aborted an SKE.

diff --git a/lib/silcske/silcske.c b/lib/silcske/silcske.c
index 00d393229239f3a2bba9520c8c4e1bfcd6f153d1..9cb98f71b5929085d2b0a2a394baabb6b00c51b6 100644 (file)
--- a/lib/silcske/silcske.c
+++ b/lib/silcske/silcske.c
@@ -73,6 +73,7 @@ static SilcBool silc_ske_packet_send(SilcSKE ske,
                                     const unsigned char *data,
                                     SilcUInt32 data_len);
 
+static void silc_ske_notify_failure(SilcSKE ske);
 /* Packet callback */
 
 static SilcBool silc_ske_packet_receive(SilcPacketEngine engine,
@@ -108,10 +109,7 @@ static SilcBool silc_ske_packet_receive(SilcPacketEngine engine,
 
   /* See if received failure from remote */
   if (packet->type == SILC_PACKET_FAILURE) {
-    if (ske->responder)
-      silc_fsm_next(&ske->fsm, silc_ske_st_responder_failure);
-    else
-      silc_fsm_next(&ske->fsm, silc_ske_st_initiator_failure);
+    silc_ske_notify_failure(ske);
   }
 
   /* Handle rekey and SUCCESS packets synchronously.  After SUCCESS packets
@@ -895,10 +893,7 @@ SILC_TASK_CALLBACK(silc_ske_packet_send_retry)
     silc_free(ske->retrans.data);
     ske->retrans.data = NULL;
     ske->status = SILC_SKE_STATUS_TIMEOUT;
-    if (ske->responder)
-      silc_fsm_next(&ske->fsm, silc_ske_st_responder_failure);
-    else
-      silc_fsm_next(&ske->fsm, silc_ske_st_initiator_failure);
+        silc_ske_notify_failure(ske);
     silc_fsm_continue_sync(&ske->fsm);
     return;
   }
@@ -986,10 +981,7 @@ SILC_TASK_CALLBACK(silc_ske_timeout)
 
   ske->packet = NULL;
   ske->status = SILC_SKE_STATUS_TIMEOUT;
-  if (ske->responder)
-    silc_fsm_next(&ske->fsm, silc_ske_st_responder_failure);
-  else
-    silc_fsm_next(&ske->fsm, silc_ske_st_initiator_failure);
+  silc_ske_notify_failure(ske);
 
   silc_fsm_continue_sync(&ske->fsm);
 }
@@ -1040,13 +1032,20 @@ void silc_ske_free(SilcSKE ske)
   SILC_LOG_DEBUG(("Freeing Key Exchange object %p: aborted=%u refcount=%hu", ske, ske->aborted, ske->refcnt));
 
     if (ske->aborted) {
-      /* If already aborted, destroy the session immediately */
-      ske->packet = NULL;
-      ske->status = SILC_SKE_STATUS_ERROR;
-      if (ske->responder)
-       silc_fsm_next(&ske->fsm, silc_ske_st_responder_failure);
-      else
-       silc_fsm_next(&ske->fsm, silc_ske_st_initiator_failure);
+      /*
+       * If already aborted, destroy the session immediately.  Only do the
+       * notification work if we have not already though, as doing so twice
+       * results in memory corruption.  We may have silc_ske_free called
+       * twice, once when the abort is requested, and then again when the
+       * FSM finish routine is called.  We have to be prepared to handle
+       * that case.
+       */
+
+      ske->packet         = NULL;
+      ske->status         = SILC_SKE_STATUS_ERROR;
+
+      silc_ske_notify_failure(ske);
+
       silc_fsm_continue_sync(&ske->fsm);
     }
 
@@ -3494,3 +3493,40 @@ SilcSKEKeyMaterial silc_ske_get_key_material(SilcSKE ske)
 {
   return ske->keymat;
 }
+
+/*
+ * Notify the owner of the ske that we failed.  Ensures that we don't make the
+ * same callout twice, as the notification callback routines are not designed
+ * to handle that case.
+ */
+static void silc_ske_notify_failure(SilcSKE ske)
+{
+  SILC_LOG_DEBUG(("Notifying SKE %p owner of failure (failure_notified = %lu)", ske, ske->failure_notified));
+
+       /*
+        * First, check if we have already made a failure callout.  If so, then we
+        * will stop here.
+        */
+
+       if (ske->failure_notified)
+               return;
+
+       /*
+        * Mark ourselves as having already sent the failure notification here and
+        * now.
+        */
+
+       ske->failure_notified = TRUE;
+
+       SILC_LOG_DEBUG(("Actually calling real failure notify callback for SKE %p (responder = %s)", ske, ske->responder ? "TRUE" : "FALSE"));
+
+       /*
+        * Finally, make the call to the owner's registered failure callback.
+        */
+
+    if (ske->responder)
+      silc_fsm_next(&ske->fsm, silc_ske_st_responder_failure);
+    else
+      silc_fsm_next(&ske->fsm, silc_ske_st_initiator_failure);
+}
+

diff --git a/lib/silcske/silcske_i.h b/lib/silcske/silcske_i.h
index e716f58d44df22dcfe66bc82398160f1112a7ae9..4473284b70d7362b0d903c69cbb384ee8ce88212 100644 (file)
--- a/lib/silcske/silcske_i.h
+++ b/lib/silcske/silcske_i.h
@@ -90,9 +90,10 @@ struct SilcSKEStruct {
   SilcUInt16 timeout;                /* SKE timeout */
   SilcUInt16 refcnt;                 /* Reference counter */
 
-  unsigned int aborted    : 1;        /* Set when SKE aborted */
-  unsigned int responder  : 1;       /* Set when we are responder side */
-  unsigned int rekeying   : 1;       /* Set when rekeying */
+  unsigned int aborted          : 1;         /* Set when SKE aborted */
+  unsigned int responder        : 1;         /* Set when we are responder side */
+  unsigned int rekeying         : 1;         /* Set when rekeying */
+  unsigned int failure_notified : 1;         /* Set to indicate that we already called the failure notify routine */
 };
 
 #endif /* SILCSKE_I_H */