From eef2f07c618c1b09976f3161e524e37cc4052499 Mon Sep 17 00:00:00 2001
From: Skywing <skywing@valhallalegends.com>
Date: Sat, 8 Nov 2008 21:19:03 -0500
Subject: [PATCH] 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.
---
 lib/silcske/silcske.c   | 74 ++++++++++++++++++++++++++++++-----------
 lib/silcske/silcske_i.h |  7 ++--
 2 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/lib/silcske/silcske.c b/lib/silcske/silcske.c
index 00d39322..9cb98f71 100644
--- 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 e716f58d..4473284b 100644
--- 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 */
-- 
2.24.0