ike-sa-manager: Avoid memory leak if IKE_SAs get checked in after flush() was called
authorTobias Brunner <tobias@strongswan.org>
Tue, 22 Mar 2016 13:22:19 +0000 (14:22 +0100)
committerTobias Brunner <tobias@strongswan.org>
Wed, 23 Mar 2016 13:02:23 +0000 (14:02 +0100)
A thread might check out a new IKE_SA via checkout_new() or
checkout_by_config() and start initiating it while the daemon is
terminating and the IKE_SA manager is flushed by the main thread.
That SA is not tracked yet so the main thread is not waiting for it and
the other thread is able to check it in and creating an entry after flush()
already terminated causing a memory leak.

Fixes #1348.

src/libcharon/sa/ike_sa_manager.c

index 307ea3b..ef85dfd 100644 (file)
@@ -2094,10 +2094,41 @@ METHOD(ike_sa_manager_t, set_spi_cb, void,
        this->spi_lock->unlock(this->spi_lock);
 }
 
+/**
+ * Destroy all entries
+ */
+static void destroy_all_entries(private_ike_sa_manager_t *this)
+{
+       enumerator_t *enumerator;
+       entry_t *entry;
+       u_int segment;
+
+       enumerator = create_table_enumerator(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);
+               }
+               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);
+       }
+       enumerator->destroy(enumerator);
+       charon->bus->set_sa(charon->bus, NULL);
+}
+
 METHOD(ike_sa_manager_t, flush, void,
        private_ike_sa_manager_t *this)
 {
-       /* destroy all list entries */
        enumerator_t *enumerator;
        entry_t *entry;
        u_int segment;
@@ -2153,27 +2184,7 @@ METHOD(ike_sa_manager_t, flush, void,
 
        DBG2(DBG_MGR, "destroy all entries");
        /* Step 4: destroy all entries */
-       enumerator = create_table_enumerator(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);
-               }
-               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);
-       }
-       enumerator->destroy(enumerator);
-       charon->bus->set_sa(charon->bus, NULL);
+       destroy_all_entries(this);
        unlock_all_segments(this);
 
        this->spi_lock->write_lock(this->spi_lock);
@@ -2189,7 +2200,11 @@ METHOD(ike_sa_manager_t, destroy, void,
 {
        u_int i;
 
-       /* these are already cleared in flush() above */
+       /* in case new SAs were checked in after flush() was called */
+       lock_all_segments(this);
+       destroy_all_entries(this);
+       unlock_all_segments(this);
+
        free(this->ike_sa_table);
        free(this->half_open_table);
        free(this->connected_peers_table);