trap-manager: Changed how acquires we acted on are tracked
This fixes potential race conditions in case complete() or flush() is executed before or concurrently with a thread that handles an acquire. It will also simplify tracking multiple acquires created for the same trap policy in the future. Also fixes the behavior in some error situations.
This commit is contained in:
Tobias Brunner
parent
773fcb1605
commit
a229bdce62
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright (C) 2011-2013 Tobias Brunner
|
||||
* Copyright (C) 2011-2015 Tobias Brunner
|
||||
* Copyright (C) 2009 Martin Willi
|
||||
* Hochschule fuer Technik Rapperswil
|
||||
*
|
||||
|
|
@ -18,10 +18,10 @@
|
|||
|
||||
#include <hydra.h>
|
||||
#include <daemon.h>
|
||||
#include <threading/mutex.h>
|
||||
#include <threading/rwlock.h>
|
||||
#include <collections/linked_list.h>
|
||||
|
||||
|
||||
typedef struct private_trap_manager_t private_trap_manager_t;
|
||||
typedef struct trap_listener_t trap_listener_t;
|
||||
|
||||
|
|
@ -66,6 +66,16 @@ struct private_trap_manager_t {
|
|||
*/
|
||||
trap_listener_t listener;
|
||||
|
||||
/**
|
||||
* list of acquires we currently handle
|
||||
*/
|
||||
linked_list_t *acquires;
|
||||
|
||||
/**
|
||||
* mutex for list of acquires
|
||||
*/
|
||||
mutex_t *mutex;
|
||||
|
||||
/**
|
||||
* Whether to ignore traffic selectors from acquires
|
||||
*/
|
||||
|
|
@ -80,23 +90,45 @@ typedef struct {
|
|||
char *name;
|
||||
/** ref to peer_cfg to initiate */
|
||||
peer_cfg_t *peer_cfg;
|
||||
/** ref to instanciated CHILD_SA */
|
||||
/** ref to instantiated CHILD_SA (i.e the trap policy) */
|
||||
child_sa_t *child_sa;
|
||||
/** TRUE if an acquire is pending */
|
||||
bool pending;
|
||||
} entry_t;
|
||||
|
||||
/**
|
||||
* A handled acquire
|
||||
*/
|
||||
typedef struct {
|
||||
/** pending IKE_SA connecting upon acquire */
|
||||
ike_sa_t *ike_sa;
|
||||
} entry_t;
|
||||
/** reqid of pending trap policy */
|
||||
u_int32_t reqid;
|
||||
} acquire_t;
|
||||
|
||||
/**
|
||||
* actually uninstall and destroy an installed entry
|
||||
*/
|
||||
static void destroy_entry(entry_t *entry)
|
||||
static void destroy_entry(entry_t *this)
|
||||
{
|
||||
entry->child_sa->destroy(entry->child_sa);
|
||||
entry->peer_cfg->destroy(entry->peer_cfg);
|
||||
free(entry->name);
|
||||
free(entry);
|
||||
this->child_sa->destroy(this->child_sa);
|
||||
this->peer_cfg->destroy(this->peer_cfg);
|
||||
free(this->name);
|
||||
free(this);
|
||||
}
|
||||
|
||||
/**
|
||||
* destroy a cached acquire entry
|
||||
*/
|
||||
static void destroy_acquire(acquire_t *this)
|
||||
{
|
||||
free(this);
|
||||
}
|
||||
|
||||
/**
|
||||
* match an acquire entry by reqid
|
||||
*/
|
||||
static bool acquire_by_reqid(acquire_t *this, u_int32_t *reqid)
|
||||
{
|
||||
return this->reqid == *reqid;
|
||||
}
|
||||
|
||||
METHOD(trap_manager_t, install, u_int32_t,
|
||||
|
|
@ -314,6 +346,7 @@ METHOD(trap_manager_t, acquire, void,
|
|||
{
|
||||
enumerator_t *enumerator;
|
||||
entry_t *entry, *found = NULL;
|
||||
acquire_t *acquire;
|
||||
peer_cfg_t *peer;
|
||||
child_cfg_t *child;
|
||||
ike_sa_t *ike_sa;
|
||||
|
|
@ -337,16 +370,29 @@ METHOD(trap_manager_t, acquire, void,
|
|||
this->lock->unlock(this->lock);
|
||||
return;
|
||||
}
|
||||
if (!cas_bool(&found->pending, FALSE, TRUE))
|
||||
reqid = found->child_sa->get_reqid(found->child_sa);
|
||||
|
||||
this->mutex->lock(this->mutex);
|
||||
if (this->acquires->find_first(this->acquires, (void*)acquire_by_reqid,
|
||||
(void**)&acquire, &reqid) == SUCCESS)
|
||||
{
|
||||
DBG1(DBG_CFG, "ignoring acquire, connection attempt pending");
|
||||
this->mutex->unlock(this->mutex);
|
||||
this->lock->unlock(this->lock);
|
||||
return;
|
||||
}
|
||||
else
|
||||
{
|
||||
INIT(acquire,
|
||||
.reqid = reqid,
|
||||
);
|
||||
this->acquires->insert_last(this->acquires, acquire);
|
||||
}
|
||||
this->mutex->unlock(this->mutex);
|
||||
|
||||
peer = found->peer_cfg->get_ref(found->peer_cfg);
|
||||
child = found->child_sa->get_config(found->child_sa);
|
||||
child = child->get_ref(child);
|
||||
reqid = found->child_sa->get_reqid(found->child_sa);
|
||||
/* don't hold the lock while checking out the IKE_SA */
|
||||
this->lock->unlock(this->lock);
|
||||
|
||||
|
|
@ -363,16 +409,13 @@ METHOD(trap_manager_t, acquire, void,
|
|||
* have a single TS that we can establish in a Quick Mode. */
|
||||
src = dst = NULL;
|
||||
}
|
||||
|
||||
this->mutex->lock(this->mutex);
|
||||
acquire->ike_sa = ike_sa;
|
||||
this->mutex->unlock(this->mutex);
|
||||
|
||||
if (ike_sa->initiate(ike_sa, child, reqid, src, dst) != DESTROY_ME)
|
||||
{
|
||||
/* make sure the entry is still there */
|
||||
this->lock->read_lock(this->lock);
|
||||
if (this->traps->find_first(this->traps, NULL,
|
||||
(void**)&found) == SUCCESS)
|
||||
{
|
||||
found->ike_sa = ike_sa;
|
||||
}
|
||||
this->lock->unlock(this->lock);
|
||||
charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);
|
||||
}
|
||||
else
|
||||
|
|
@ -381,6 +424,14 @@ METHOD(trap_manager_t, acquire, void,
|
|||
ike_sa);
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
this->mutex->lock(this->mutex);
|
||||
this->acquires->remove(this->acquires, acquire, NULL);
|
||||
this->mutex->unlock(this->mutex);
|
||||
destroy_acquire(acquire);
|
||||
child->destroy(child);
|
||||
}
|
||||
peer->destroy(peer);
|
||||
}
|
||||
|
||||
|
|
@ -391,26 +442,25 @@ static void complete(private_trap_manager_t *this, ike_sa_t *ike_sa,
|
|||
child_sa_t *child_sa)
|
||||
{
|
||||
enumerator_t *enumerator;
|
||||
entry_t *entry;
|
||||
acquire_t *acquire;
|
||||
|
||||
this->lock->read_lock(this->lock);
|
||||
enumerator = this->traps->create_enumerator(this->traps);
|
||||
while (enumerator->enumerate(enumerator, &entry))
|
||||
this->mutex->lock(this->mutex);
|
||||
enumerator = this->acquires->create_enumerator(this->acquires);
|
||||
while (enumerator->enumerate(enumerator, &acquire))
|
||||
{
|
||||
if (entry->ike_sa != ike_sa)
|
||||
if (!acquire->ike_sa || acquire->ike_sa != ike_sa)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if (child_sa && child_sa->get_reqid(child_sa) !=
|
||||
entry->child_sa->get_reqid(entry->child_sa))
|
||||
if (child_sa && child_sa->get_reqid(child_sa) != acquire->reqid)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
entry->ike_sa = NULL;
|
||||
entry->pending = FALSE;
|
||||
this->acquires->remove_at(this->acquires, enumerator);
|
||||
destroy_acquire(acquire);
|
||||
}
|
||||
enumerator->destroy(enumerator);
|
||||
this->lock->unlock(this->lock);
|
||||
this->mutex->unlock(this->mutex);
|
||||
}
|
||||
|
||||
METHOD(listener_t, ike_state_change, bool,
|
||||
|
|
@ -444,14 +494,10 @@ METHOD(listener_t, child_state_change, bool,
|
|||
METHOD(trap_manager_t, flush, void,
|
||||
private_trap_manager_t *this)
|
||||
{
|
||||
linked_list_t *traps;
|
||||
/* since destroying the CHILD_SA results in events which require a read
|
||||
* lock we cannot destroy the list while holding the write lock */
|
||||
this->lock->write_lock(this->lock);
|
||||
traps = this->traps;
|
||||
this->traps->destroy_function(this->traps, (void*)destroy_entry);
|
||||
this->traps = linked_list_create();
|
||||
this->lock->unlock(this->lock);
|
||||
traps->destroy_function(traps, (void*)destroy_entry);
|
||||
}
|
||||
|
||||
METHOD(trap_manager_t, destroy, void,
|
||||
|
|
@ -459,6 +505,8 @@ METHOD(trap_manager_t, destroy, void,
|
|||
{
|
||||
charon->bus->remove_listener(charon->bus, &this->listener.listener);
|
||||
this->traps->destroy_function(this->traps, (void*)destroy_entry);
|
||||
this->acquires->destroy_function(this->acquires, (void*)destroy_acquire);
|
||||
this->mutex->destroy(this->mutex);
|
||||
this->lock->destroy(this->lock);
|
||||
free(this);
|
||||
}
|
||||
|
|
@ -488,6 +536,8 @@ trap_manager_t *trap_manager_create(void)
|
|||
},
|
||||
},
|
||||
.traps = linked_list_create(),
|
||||
.acquires = linked_list_create(),
|
||||
.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
|
||||
.lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
|
||||
.ignore_acquire_ts = lib->settings->get_bool(lib->settings,
|
||||
"%s.ignore_acquire_ts", FALSE, lib->ns),
|
||||
|
|
|
|||
Loading…
Reference in New Issue