don't send DPD messages before the exchange is finialized, otherwise

we have a race between DPD and exchange timeouts and both will release
the SA and corrupt the SA list. ok hshoexer@, ho@

3 files changed, 26 insertions, 10 deletions

diff --git a/sbin/isakmpd/dpd.c b/sbin/isakmpd/dpd.c
index 040b030a3b6..61fdd63843e 100644
--- a/sbin/isakmpd/dpd.c
+++ b/sbin/isakmpd/dpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dpd.c,v 1.13 2005/05/04 10:05:01 hshoexer Exp $	*/
+/*	$OpenBSD: dpd.c,v 1.14 2005/10/26 20:10:48 markus Exp $	*/
 
 /*
  * Copyright (c) 2004 Håkan Olsson.  All rights reserved.
@@ -122,19 +122,25 @@ dpd_check_vendor_payload(struct message *msg, struct payload *p)
 			msg->exchange->flags |= EXCHANGE_FLAG_DPD_CAP_PEER;
 			LOG_DBG((LOG_EXCHANGE, 10, "dpd_check_vendor_payload: "
 			    "DPD capable peer detected"));
-			if (dpd_timer_interval(0) != 0) {
-				LOG_DBG((LOG_EXCHANGE, 10,
-				    "dpd_check_vendor_payload: enabling"));
-				msg->isakmp_sa->flags |= SA_FLAG_DPD;
-				dpd_timer_reset(msg->isakmp_sa, 0,
-				    DPD_TIMER_NORMAL);
-			}
 		}
 		p->flags |= PL_MARK;
 	}
 }
 
 /*
+ * Arm the DPD timer
+ */
+void
+dpd_start(struct sa *isakmp_sa)
+{
+	if (dpd_timer_interval(0) != 0) {
+		LOG_DBG((LOG_EXCHANGE, 10, "dpd_enable: enabling"));
+		isakmp_sa->flags |= SA_FLAG_DPD;
+		dpd_timer_reset(isakmp_sa, 0, DPD_TIMER_NORMAL);
+	}
+}
+
+/*
  * All incoming DPD Notify messages enter here. Message has been validated.
  */
 void
diff --git a/sbin/isakmpd/dpd.h b/sbin/isakmpd/dpd.h
index fa62c5b08b7..d017d403fbf 100644
--- a/sbin/isakmpd/dpd.h
+++ b/sbin/isakmpd/dpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dpd.h,v 1.2 2004/08/10 15:59:10 ho Exp $	*/
+/*	$OpenBSD: dpd.h,v 1.3 2005/10/26 20:10:49 markus Exp $	*/
 
 /*
  * Copyright (c) 2004 Håkan Olsson.  All rights reserved.
@@ -34,5 +34,6 @@ struct sa;
 int	dpd_add_vendor_payload(struct message *);
 void	dpd_check_vendor_payload(struct message *, struct payload *);
 void	dpd_handle_notify(struct message *, struct payload *);
+void	dpd_start(struct sa *);
 
 #endif /* _DPD_H_ */
diff --git a/sbin/isakmpd/exchange.c b/sbin/isakmpd/exchange.c
index 566b3dd45c4..fa03cbc6507 100644
--- a/sbin/isakmpd/exchange.c
+++ b/sbin/isakmpd/exchange.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: exchange.c,v 1.123 2005/07/05 11:57:03 hshoexer Exp $	 */
+/* $OpenBSD: exchange.c,v 1.124 2005/10/26 20:10:49 markus Exp $	 */
 /* $EOM: exchange.c,v 1.143 2000/12/04 00:02:25 angelos Exp $	 */
 
 /*
@@ -60,6 +60,7 @@
 #include "sa.h"
 #include "util.h"
 #include "key.h"
+#include "dpd.h"
 
 /* Initial number of bits from the cookies used as hash.  */
 #define INITIAL_BUCKET_BITS 6
@@ -1477,6 +1478,14 @@ exchange_finalize(struct message *msg)
 		TAILQ_REMOVE(&exchange->sa_list, sa, next);
 		sa_release(sa);
 	}
+	/*
+	 * Start sending DPD messages after all SAs have been released.
+	 * Otherwise we have a race between exchange_free_aux() and
+	 * dpd_check_event() where both will call sa_free().
+	 */
+	if (exchange->phase == 1 && msg->isakmp_sa &&
+	    (exchange->flags & EXCHANGE_FLAG_DPD_CAP_PEER))
+		dpd_start(msg->isakmp_sa);
 
 	/* If we have nothing to retransmit we can safely remove ourselves.  */
 	if (!exchange->last_sent)