sharpetronics/strongswan
1
0
Fork 0
mirror of https://github.com/strongswan/strongswan.git synced 2025-10-03 00:00:24 -04:00
Code Issues Projects Releases Wiki Activity

ike-sa-manager: Make sure flush() removes entries that might get added concurrently

Browse Source 
Because flush() has to release the segment locks intermittently, threads
might add new entries (even with the change in the previous commit as the
IKE_SA might already be created, just not registered/checked in yet).

Since those entries are added to the front of the segment lists, the
enumerator in the previous step 2 didn't notice them and did not wait
for them to get checked in.  However, step 3 and 4 then proceeded to
delete and destroy the entry and IKE_SA, which could lead to a crash
once the other thread attempts to check in the already destroyed IKE_SA.

This change combines the three loops of steps 2-4 but then loops over
the whole table until it's actually empty.  This way we wait for and
destroy newly added entries.
This commit is contained in:
Tobias Brunner 2022-08-26 16:14:30 +02:00
parent 6f456afe39
commit 2740c50bb8
1 changed files with 69 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
src/libcharon/sa/ike_sa_manager.c
View File

@ -1,5 +1,5 @@
/*
* Copyright (C) 2008-2021 Tobias Brunner
* Copyright (C) 2008-2022 Tobias Brunner
* Copyright (C) 2005-2011 Martin Willi
* Copyright (C) 2005 Jan Hutter
*
@ -65,14 +65,9 @@ struct entry_t {
thread_t *checked_out;
/**
* Does this SA drives out new threads?
* Whether threads are prevented from getting this IKE_SA.
*/
bool driveout_new_threads;
/**
* Does this SA drives out waiting threads?
*/
bool driveout_waiting_threads;
bool driveout_threads;
/**
* Identification of an IKE_SA (SPIs).
@ -350,6 +345,12 @@ struct private_ike_sa_manager_t {
*/
u_int segment_mask;
/**
* Enabled while shutting down to mark all new entries so no threads
* will check them out.
*/
bool new_entries_driveout_threads;
/**
* Hash table with half_open_t objects.
*/
@ -631,6 +632,8 @@ static u_int put_entry(private_ike_sa_manager_t *this, entry_t *entry)
segment = row & this->segment_mask;
lock_single_segment(this, segment);
/* mark entry during shutdown */
entry->driveout_threads = this->new_entries_driveout_threads;
current = this->ike_sa_table[row];
if (current)
{ /* insert at the front of current bucket */
@ -759,12 +762,12 @@ static status_t get_entry_by_sa(private_ike_sa_manager_t *this,
static bool wait_for_entry(private_ike_sa_manager_t *this, entry_t *entry,
u_int segment)
{
if (entry->driveout_new_threads)
if (entry->driveout_threads)
{
/* we are not allowed to get this */
return FALSE;
}
while (entry->checked_out && !entry->driveout_waiting_threads)
while (entry->checked_out && !entry->driveout_threads)
{
/* so wait until we can get it for us.
* we register us as waiting. */
@ -773,7 +776,7 @@ static bool wait_for_entry(private_ike_sa_manager_t *this, entry_t *entry,
entry->waiting_threads--;
}
/* hm, a deletion request forbids us to get this SA, get next one */
if (entry->driveout_waiting_threads)
if (entry->driveout_threads)
{
/* we must signal here, others may be waiting on it, too */
entry->condvar->signal(entry->condvar);
@ -1761,7 +1764,7 @@ METHOD(ike_sa_manager_t, new_initiator_spi, bool,
if (get_entry_by_sa(this, ike_sa_id, ike_sa, &entry, &segment) == SUCCESS)
{
if (entry->driveout_waiting_threads && entry->driveout_new_threads)
if (entry->driveout_threads)
{ /* it looks like flush() has been called and the SA is being deleted
* anyway, no need for a new SPI */
DBG2(DBG_MGR, "ignored change of initiator SPI during shutdown");
@ -1833,8 +1836,7 @@ CALLBACK(enumerator_filter_skip, bool,
while (orig->enumerate(orig, &entry, &segment))
{
if (!entry->driveout_new_threads &&
!entry->driveout_waiting_threads &&
if (!entry->driveout_threads &&
!entry->checked_out)
{
*out = entry->ike_sa;
@ -1997,7 +1999,7 @@ METHOD(ike_sa_manager_t, checkin_and_destroy, void,
if (get_entry_by_sa(this, ike_sa_id, ike_sa, &entry, &segment) == SUCCESS)
{
if (entry->driveout_waiting_threads && entry->driveout_new_threads)
if (entry->driveout_threads)
{ /* it looks like flush() has been called and the SA is being deleted
* anyway, just check it in */
DBG2(DBG_MGR, "ignored checkin and destroy of IKE_SA during shutdown");
@ -2007,10 +2009,8 @@ METHOD(ike_sa_manager_t, checkin_and_destroy, void,
return;
}
/* drive out waiting threads, as we are in hurry */
entry->driveout_waiting_threads = TRUE;
/* mark it, so no new threads can get this entry */
entry->driveout_new_threads = TRUE;
/* mark it, so no threads can get this entry */
entry->driveout_threads = TRUE;
/* wait until all workers have done their work */
while (entry->waiting_threads)
{
@ -2351,6 +2351,29 @@ METHOD(ike_sa_manager_t, set_spi_cb, void,
this->spi_lock->unlock(this->spi_lock);
}
/**
* Destroy a single entry
*/
static void remove_and_destroy_entry(private_ike_sa_manager_t *this,
enumerator_t *enumerator, entry_t *entry)
{
if (entry->half_open)
{
remove_half_open(this, entry->other,
entry->ike_sa_id->is_initiator(entry->ike_sa_id));
}
if (entry->my_id && entry->other_id)
{
remove_connected_peers(this, entry);
}
if (entry->init_hash.ptr)
{
remove_init_hash(this, entry->init_hash);
}
remove_entry_at((private_enumerator_t*)enumerator);
entry_destroy(entry);
}
/**
* Destroy all entries
*/
@ -2364,21 +2387,7 @@ static void destroy_all_entries(private_ike_sa_manager_t *this)
while (enumerator->enumerate(enumerator, &entry, &segment))
{
charon->bus->set_sa(charon->bus, entry->ike_sa);
if (entry->half_open)
{
remove_half_open(this, entry->other,
entry->ike_sa_id->is_initiator(entry->ike_sa_id));
}
if (entry->my_id && entry->other_id)
{
remove_connected_peers(this, entry);
}
if (entry->init_hash.ptr)
{
remove_init_hash(this, entry->init_hash);
}
remove_entry_at((private_enumerator_t*)enumerator);
entry_destroy(entry);
remove_and_destroy_entry(this, enumerator, entry);
}
enumerator->destroy(enumerator);
charon->bus->set_sa(charon->bus, NULL);
@ -2400,44 +2409,41 @@ METHOD(ike_sa_manager_t, flush, void,
this->spi_lock->unlock(this->spi_lock);
lock_all_segments(this);
DBG2(DBG_MGR, "going to destroy IKE_SA manager and all managed IKE_SA's");
/* Step 1: drive out all waiting threads */
DBG2(DBG_MGR, "set driveout flags for all stored IKE_SA's");
DBG2(DBG_MGR, "going to destroy IKE_SA manager and all managed IKE_SAs");
enumerator = create_table_enumerator(this);
while (enumerator->enumerate(enumerator, &entry, &segment))
{
/* do not accept new threads, drive out waiting threads */
entry->driveout_new_threads = TRUE;
entry->driveout_waiting_threads = TRUE;
/* mark all entries so threads can't check these IKE_SAs out */
entry->driveout_threads = TRUE;
}
enumerator->destroy(enumerator);
DBG2(DBG_MGR, "wait for all threads to leave IKE_SA's");
/* Step 2: wait until all are gone */
enumerator = create_table_enumerator(this);
while (enumerator->enumerate(enumerator, &entry, &segment))
DBG2(DBG_MGR, "wait for threads to leave IKE_SAs and delete and destroy "
"them");
/* we are intermittently unlocking the segments below, which allows threads
* to create new entries. we want these to get marked directly so other
* threads can't get them again. we re-enumerate the table until all
* entries are gone */
this->new_entries_driveout_threads = TRUE;
while (ref_cur(&this->total_sa_count))
{
while (entry->waiting_threads || entry->checked_out)
enumerator = create_table_enumerator(this);
while (enumerator->enumerate(enumerator, &entry, &segment))
{
/* wake up all */
entry->condvar->broadcast(entry->condvar);
/* go sleeping until they are gone */
entry->condvar->wait(entry->condvar, this->segments[segment].mutex);
while (entry->waiting_threads || entry->checked_out)
{
/* wake up all */
entry->condvar->broadcast(entry->condvar);
/* go sleeping until they are gone */
entry->condvar->wait(entry->condvar,
this->segments[segment].mutex);
}
charon->bus->set_sa(charon->bus, entry->ike_sa);
entry->ike_sa->delete(entry->ike_sa, TRUE);
remove_and_destroy_entry(this, enumerator, entry);
charon->bus->set_sa(charon->bus, NULL);
}
enumerator->destroy(enumerator);
}
enumerator->destroy(enumerator);
DBG2(DBG_MGR, "delete all IKE_SA's");
/* Step 3: initiate deletion of all IKE_SAs */
enumerator = create_table_enumerator(this);
while (enumerator->enumerate(enumerator, &entry, &segment))
{
charon->bus->set_sa(charon->bus, entry->ike_sa);
entry->ike_sa->delete(entry->ike_sa, TRUE);
}
enumerator->destroy(enumerator);
DBG2(DBG_MGR, "destroy all entries");
/* Step 4: destroy all entries */
destroy_all_entries(this);
unlock_all_segments(this);
}