]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
ike-sa-manager: Avoid deadlock due to race condition during shutdown
authorTobias Brunner <tobias@strongswan.org>
Mon, 11 Aug 2025 12:24:16 +0000 (14:24 +0200)
committerTobias Brunner <tobias@strongswan.org>
Fri, 22 Aug 2025 13:26:59 +0000 (15:26 +0200)
If an entry is added while we wait for a checked out SA in flush() (e.g.
due to an action performed by that SA), new entries might get inserted
before the one we wait for.  If that was the first entry in the row, we
didn't correctly update the table and the new entries were basically lost
by overwriting the first entry in the row.  As the SA count was still
increased but the new entries couldn't get enumerated, the daemon wasn't
terminated properly but was stuck in the loop in flush().

src/libcharon/sa/ike_sa_manager.c

index fca61ce7c8b2b3fa45244859a99bd8b5c5644a46..7796efb658e2c80a250992d3c037f5c924071778 100644 (file)
@@ -677,14 +677,15 @@ static void remove_entry(private_ike_sa_manager_t *this, entry_t *entry)
 }
 
 /**
- * Remove the entry at the current enumerator position.
+ * Remove the entry at the current enumerator position. This only works for a
+ * single concurrent thread.
  */
 static void remove_entry_at(private_enumerator_t *this)
 {
        this->entry = NULL;
        if (this->current)
        {
-               table_item_t *current = this->current;
+               table_item_t *current = this->current, *prev;
 
                ignore_result(ref_put(&this->manager->total_sa_count));
                this->current = this->prev;
@@ -695,7 +696,22 @@ static void remove_entry_at(private_enumerator_t *this)
                }
                else
                {
-                       this->manager->ike_sa_table[this->row] = current->next;
+                       /* we started from the beginning of the row, but while we waited
+                        * for the entry in flush(), one or more entries might have been
+                        * added, start from the beginning again in either case */
+                       prev = this->manager->ike_sa_table[this->row];
+                       if (prev != current)
+                       {
+                               while (prev->next != current)
+                               {
+                                       prev = prev->next;
+                               }
+                               prev->next = current->next;
+                       }
+                       else
+                       {
+                               this->manager->ike_sa_table[this->row] = current->next;
+                       }
                        unlock_single_segment(this->manager, this->segment);
                }
                free(current);