From 12b3cdba7689113558f58a5265827f3086852bae Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Mon, 13 Jul 2015 13:20:14 +0200
Subject: [PATCH] trap-manager: Resolve race conditions between flush() and
 install()

When flush() is called there might be threads in install() waiting for
trap policies to get installed (without holding the lock).  We have to
wait until they updated the entries with the respective CHILD_SAs before
destroying the list.

We also have to prevent further trap policy installations (and wait until
threads in install() are really finished), otherwise we might end up
destroying CHILD_SA objects after the kernel interface implementations
have already been unloaded (avoiding this is the whole point of calling
flush() before unloading the plugins).
---
 src/libcharon/sa/trap_manager.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 83b6d6a..424d9e7 100644
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -20,8 +20,11 @@
 #include <daemon.h>
 #include <threading/mutex.h>
 #include <threading/rwlock.h>
+#include <threading/rwlock_condvar.h>
 #include <collections/linked_list.h>
 
+#define INSTALL_DISABLED ((u_int)~0)
+
 typedef struct private_trap_manager_t private_trap_manager_t;
 typedef struct trap_listener_t trap_listener_t;
 
@@ -77,6 +80,16 @@ struct private_trap_manager_t {
 	mutex_t *mutex;
 
 	/**
+	 * number of threads currently installing trap policies, or INSTALL_DISABLED
+	 */
+	u_int installing;
+
+	/**
+	 * condvar to signal trap policy installation
+	 */
+	rwlock_condvar_t *condvar;
+
+	/**
 	 * Whether to ignore traffic selectors from acquires
 	 */
 	bool ignore_acquire_ts;
@@ -171,6 +184,11 @@ METHOD(trap_manager_t, install, u_int32_t,
 	}
 
 	this->lock->write_lock(this->lock);
+	if (this->installing == INSTALL_DISABLED)
+	{	/* flush() has been called */
+		this->lock->unlock(this->lock);
+		return 0;
+	}
 	enumerator = this->traps->create_enumerator(this->traps);
 	while (enumerator->enumerate(enumerator, &entry))
 	{
@@ -204,6 +222,7 @@ METHOD(trap_manager_t, install, u_int32_t,
 		.peer_cfg = peer->get_ref(peer),
 	);
 	this->traps->insert_first(this->traps, entry);
+	this->installing++;
 	/* don't hold lock while creating CHILD_SA and installing policies */
 	this->lock->unlock(this->lock);
 
@@ -252,6 +271,11 @@ METHOD(trap_manager_t, install, u_int32_t,
 	{
 		destroy_entry(found);
 	}
+	this->lock->write_lock(this->lock);
+	/* do this at the end, so entries created temporarily are also destroyed */
+	this->installing--;
+	this->condvar->signal(this->condvar);
+	this->lock->unlock(this->lock);
 	return reqid;
 }
 
@@ -495,8 +519,13 @@ METHOD(trap_manager_t, flush, void,
 	private_trap_manager_t *this)
 {
 	this->lock->write_lock(this->lock);
+	while (this->installing)
+	{
+		this->condvar->wait(this->condvar, this->lock);
+	}
 	this->traps->destroy_function(this->traps, (void*)destroy_entry);
 	this->traps = linked_list_create();
+	this->installing = INSTALL_DISABLED;
 	this->lock->unlock(this->lock);
 }
 
@@ -506,6 +535,7 @@ 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->condvar->destroy(this->condvar);
 	this->mutex->destroy(this->mutex);
 	this->lock->destroy(this->lock);
 	free(this);
@@ -539,6 +569,7 @@ trap_manager_t *trap_manager_create(void)
 		.acquires = linked_list_create(),
 		.mutex = mutex_create(MUTEX_TYPE_DEFAULT),
 		.lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
+		.condvar = rwlock_condvar_create(),
 		.ignore_acquire_ts = lib->settings->get_bool(lib->settings,
 										"%s.ignore_acquire_ts", FALSE, lib->ns),
 	);
-- 
2.4.6