diff options
author | Tobias Brunner <tobias@strongswan.org> | 2015-09-02 12:14:35 +0200 |
---|---|---|
committer | Tobias Brunner <tobias@strongswan.org> | 2015-10-29 15:59:41 +0100 |
commit | 758b1caa0e75b6de011057fbc8c2982303745953 (patch) | |
tree | 1db805541297b0c5949f1f825c03bfaa619bd0cd | |
parent | 858148092d1ef755cd40fea0169259e4ad3e9f02 (diff) | |
download | strongswan-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.c | 16 |
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; } } |