aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTobias Brunner <tobias@strongswan.org>2015-09-02 12:14:35 +0200
committerTobias Brunner <tobias@strongswan.org>2015-10-29 15:59:41 +0100
commit758b1caa0e75b6de011057fbc8c2982303745953 (patch)
tree1db805541297b0c5949f1f825c03bfaa619bd0cd
parent858148092d1ef755cd40fea0169259e4ad3e9f02 (diff)
downloadstrongswan-758b1caa0e75.tar.bz2
strongswan-758b1caa0e75.tar.xz
ikev1: Prevent deadlock when checking for duplicate IKEv1 SAs
Previously, the current segment was held while checking for duplicate SAs, which requires acquiring all segments. If multiple threads did this concurrently this resulted in a deadlock as they couldn't acquire the segments held by the other threads attempting to do the same. With the default configuration only one segment is used, which prevents the problem as only one thread can check in an IKE SA concurrently. Fixes: a064eaa8a63a ("Handling of initial contact")
-rw-r--r--src/libcharon/sa/ike_sa_manager.c16
1 files changed, 16 insertions, 0 deletions
diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c
index 37d69874d..9ebdae7f6 100644
--- a/src/libcharon/sa/ike_sa_manager.c
+++ b/src/libcharon/sa/ike_sa_manager.c
@@ -1628,8 +1628,24 @@ METHOD(ike_sa_manager_t, checkin, void,
* delete any existing IKE_SAs with that peer. */
if (ike_sa->has_condition(ike_sa, COND_INIT_CONTACT_SEEN))
{
+ /* We can't hold the segment locked while checking the
+ * uniqueness as this could lead to deadlocks. We mark the
+ * entry as checked out while we release the lock so no other
+ * thread can acquire it. Since it is not yet in the list of
+ * connected peers that will not cause a deadlock as no other
+ * caller of check_unqiueness() will try to check out this SA */
+ entry->checked_out = TRUE;
+ unlock_single_segment(this, segment);
+
this->public.check_uniqueness(&this->public, ike_sa, TRUE);
ike_sa->set_condition(ike_sa, COND_INIT_CONTACT_SEEN, FALSE);
+
+ /* The entry could have been modified in the mean time, e.g.
+ * because another SA was added/removed next to it or another
+ * thread is waiting, but it should still exist, so there is no
+ * need for a lookup via get_entry_by... */
+ lock_single_segment(this, segment);
+ entry->checked_out = FALSE;
}
}