ims-volte-vowifi
/
strongswan
9
0
Fork 0
Code Pull Requests Releases Activity

kernel-netlink: Let only a single thread work on a specific policy 

Other threads are free to add/update/delete other policies.

This tries to prevent race conditions caused by releasing the mutex while
sending messages to the kernel.  For instance, if break-before-make
reauthentication is used and one thread on the responder is delayed in
deleting the policies that another thread is concurrently adding for the
new SA.  This could have resulted in no policies being installed
eventually.

Fixes #1400.
This commit is contained in:
Tobias Brunner 2016-05-25 12:15:38 +02:00
parent 471b907613
commit ebeaac1f2b
1 changed files with 69 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
View File

@ -41,6 +41,7 @@
#include <daemon.h>
#include <utils/debug.h>
#include <threading/mutex.h>
#include <threading/condvar.h>
#include <collections/array.h>
#include <collections/hashtable.h>
#include <collections/linked_list.h>
@ -289,6 +290,11 @@ struct private_kernel_netlink_ipsec_t {
*/
mutex_t *mutex;
/**
* Condvar to synchronize access to individual policies
*/
condvar_t *condvar;
/**
* Hash table of installed policies (policy_entry_t)
*/
@ -575,6 +581,12 @@ struct policy_entry_t {
/** reqid for this policy */
uint32_t reqid;
/** Number of threads waiting to work on this policy */
int waiting;
/** TRUE if a thread is working on this policy */
bool working;
};
/**
@ -2148,6 +2160,20 @@ METHOD(kernel_ipsec_t, flush_sas, status_t,
return SUCCESS;
}
/**
* Unlock the mutex and signal waiting threads
*/
static void policy_change_done(private_kernel_netlink_ipsec_t *this,
policy_entry_t *policy)
{
policy->working = FALSE;
if (policy->waiting)
{ /* don't need to wake threads waiting for other policies */
this->condvar->broadcast(this->condvar);
}
this->mutex->unlock(this->mutex);
}
/**
* Add or update a policy in the kernel.
*
@ -2219,7 +2245,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
count * sizeof(*tmpl));
if (!tmpl)
{
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
return FAILED;
}
@ -2252,7 +2278,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
if (!add_mark(hdr, sizeof(request), ipsec->mark))
{
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
return FAILED;
}
this->mutex->unlock(this->mutex);
@ -2264,22 +2290,13 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
hdr->nlmsg_type = XFRM_MSG_UPDPOLICY;
status = this->socket_xfrm->send_ack(this->socket_xfrm, hdr);
}
this->mutex->lock(this->mutex);
if (status != SUCCESS)
{
policy_change_done(this, policy);
return FAILED;
}
/* find the policy again */
this->mutex->lock(this->mutex);
policy = this->policies->get(this->policies, &clone);
if (!policy ||
policy->used_by->find_first(policy->used_by,
NULL, (void**)&mapping) != SUCCESS)
{ /* policy or mapping is already gone, ignore */
this->mutex->unlock(this->mutex);
return SUCCESS;
}
/* install a route, if:
* - this is a inbound policy (to just get one for each child)
* - we are in tunnel/BEET mode or install a bypass policy
@ -2328,7 +2345,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
if (!charon->kernel->get_interface(charon->kernel, iface,
&route->if_name))
{
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
route_entry_destroy(route);
return SUCCESS;
}
@ -2338,7 +2355,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
route_entry_t *old = policy->route;
if (route_entry_equals(old, route))
{
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
route_entry_destroy(route);
return SUCCESS;
}
@ -2380,7 +2397,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this,
free(route);
}
}
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
return SUCCESS;
}
@ -2428,6 +2445,14 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
policy_entry_destroy(this, policy);
policy = current;
found = TRUE;
policy->waiting++;
while (policy->working)
{
this->condvar->wait(this->condvar, this->mutex);
}
policy->waiting--;
policy->working = TRUE;
}
else
{ /* use the new one, if we have no such policy */
@ -2478,7 +2503,7 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
if (!update)
{ /* we don't update the policy if the priority is lower than that of
* the currently installed one */
this->mutex->unlock(this->mutex);
policy_change_done(this, policy);
DBG2(DBG_KNL, "not updating policy %R === %R %N%s [priority %u,"
"refcount %d]", id->src_ts, id->dst_ts, policy_dir_names,
id->dir, markstr, cur_priority, use_count);
@ -2606,6 +2631,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
};
char markstr[32] = "";
int use_count;
status_t status = SUCCESS;
format_mark(markstr, sizeof(markstr), id->mark);
@ -2628,6 +2654,13 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
this->mutex->unlock(this->mutex);
return NOT_FOUND;
}
current->waiting++;
while (current->working)
{
this->condvar->wait(this->condvar, this->mutex);
}
current->working = TRUE;
current->waiting--;
/* remove mapping to SA by reqid and priority */
auto_priority = get_priority(current, data->prio,id->interface);
@ -2661,7 +2694,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
DBG2(DBG_KNL, "policy still used by another CHILD_SA, not removed");
if (!is_installed)
{ /* no need to update as the policy was not installed for this SA */
this->mutex->unlock(this->mutex);
policy_change_done(this, current);
DBG2(DBG_KNL, "not updating policy %R === %R %N%s [priority %u, "
"refcount %d]", id->src_ts, id->dst_ts, policy_dir_names,
id->dir, markstr, cur_priority, use_count);
@ -2695,7 +2728,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
if (!add_mark(hdr, sizeof(request), id->mark))
{
this->mutex->unlock(this->mutex);
policy_change_done(this, current);
return FAILED;
}
@ -2711,18 +2744,27 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
id->dir, markstr);
}
}
this->policies->remove(this->policies, current);
policy_entry_destroy(this, current);
this->mutex->unlock(this->mutex);
if (this->socket_xfrm->send_ack(this->socket_xfrm, hdr) != SUCCESS)
{
DBG1(DBG_KNL, "unable to delete policy %R === %R %N%s", id->src_ts,
id->dst_ts, policy_dir_names, id->dir, markstr);
return FAILED;
status = FAILED;
}
return SUCCESS;
this->mutex->lock(this->mutex);
if (!current->waiting)
{ /* only if no other thread still needs the policy */
this->policies->remove(this->policies, current);
policy_entry_destroy(this, current);
this->mutex->unlock(this->mutex);
}
else
{
policy_change_done(this, current);
}
return status;
}
METHOD(kernel_ipsec_t, flush_policies, status_t,
@ -2978,6 +3020,7 @@ METHOD(kernel_ipsec_t, destroy, void,
enumerator->destroy(enumerator);
this->policies->destroy(this->policies);
this->sas->destroy(this->sas);
this->condvar->destroy(this->condvar);
this->mutex->destroy(this->mutex);
free(this);
}
@ -3017,6 +3060,7 @@ kernel_netlink_ipsec_t *kernel_netlink_ipsec_create()
(hashtable_equals_t)ipsec_sa_equals, 32),
.bypass = array_create(sizeof(bypass_t), 0),
.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
.condvar = condvar_create(CONDVAR_TYPE_DEFAULT),
.get_priority = dlsym(RTLD_DEFAULT,
"kernel_netlink_get_priority_custom"),
.policy_update = lib->settings->get_bool(lib->settings,