[Openvpn-devel,v1] multi: release the session-id hash reference on shutdown

Message ID 20261026110324.55339-1-frank@lichtenheld.com
State New
Headers
Series [Openvpn-devel,v1] multi: release the session-id hash reference on shutdown |

Commit Message

Frank Lichtenheld Oct. 26, 2026, 11:03 a.m. UTC
  From: Antonio Quartulli <antonio@mandelbit.com>

multi_hash_sid_add() takes a reference on the multi_instance, but
multi_close_instance() only drops it via multi_hash_sid_remove() when
not shutting down. On shutdown hash_free() then frees the table without
releasing the reference, so every instance still connected when the
server stops is leaked (reported by LeakSanitizer, which also turns the
exit code into 1).

Remove the instance from the session-id hash regardless of shutdown.
Unlike the other tables handled in that block, this one holds a
reference.

Fixes: 66f8cdb9 ("Add lookup of multi session by session id")
Change-Id: I62d95dad21891351a16cd6be43063406a024acee
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2032
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2032
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>
  

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index e302a81..813b79c 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -584,6 +584,13 @@ 
         m->earliest_wakeup = NULL;
     }
 
+    /* The sid hash holds a reference on the instance: drop it on shutdown
+     * too, otherwise the instance is never freed */
+    if (session_id_defined(&mi->sid_hashed_value))
+    {
+        multi_hash_sid_remove(m, &mi->sid_hashed_value);
+    }
+
     if (!shutdown)
     {
         if (mi->did_real_hash)
@@ -605,12 +612,6 @@ 
         }
 #endif
 
-
-        if (session_id_defined(&mi->sid_hashed_value))
-        {
-            multi_hash_sid_remove(m, &mi->sid_hashed_value);
-        }
-
         if (mi->context.c2.tls_multi->rx_peer_id != MAX_PEER_ID)
         {
             m->instances[mi->context.c2.tls_multi->rx_peer_id] = NULL;