From 058e390ab5d40e5322b2d082c0e15b0dbde2c1d5 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Thu, 1 Jul 2021 18:59:44 +0100
Subject: [PATCH] Avoid crash in MIT Kerberos for Windows on session restart.

A user reports that if you have MIT KfW loaded, and your PuTTY session
terminates without the PuTTY process exiting, and you select 'Restart
Session' from the menu, then a crash occurs inside the Kerberos
library itself. Scuttlebutt on the Internet suggested this might be to
do with unloading and then reloading the DLL within the process
lifetime, which indeed we were doing.

Now we avoid doing that for the KfW library in particular, by keeping
a tree234 of module handles marked 'never unload this'.

This is a workaround at best, but it seems to stop the problem
happening in my own tests.
---
 windows/gss.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/windows/gss.c b/windows/gss.c
index 0b47d9a7..ebe5698d 100644
--- a/windows/gss.c
+++ b/windows/gss.c
@@ -95,6 +95,28 @@ const char *gsslogmsg = NULL;
 
 static void ssh_sspi_bind_fns(struct ssh_gss_library *lib);
 
+static tree234 *libraries_to_never_unload;
+static int library_to_never_unload_cmp(void *av, void *bv)
+{
+    uintptr_t a = (uintptr_t)av, b = (uintptr_t)bv;
+    return a < b ? -1 : a > b ? +1 : 0;
+}
+static void ensure_library_tree_exists(void)
+{
+    if (!libraries_to_never_unload)
+        libraries_to_never_unload = newtree234(library_to_never_unload_cmp);
+}
+static bool library_is_in_never_unload_tree(HMODULE module)
+{
+    ensure_library_tree_exists();
+    return find234(libraries_to_never_unload, module, NULL);
+}
+static void add_library_to_never_unload_tree(HMODULE module)
+{
+    ensure_library_tree_exists();
+    add234(libraries_to_never_unload, module);
+}
+
 struct ssh_gss_liblist *ssh_gss_setup(Conf *conf)
 {
     HMODULE module;
@@ -145,6 +167,23 @@ struct ssh_gss_liblist *ssh_gss_setup(Conf *conf)
                                         LOAD_LIBRARY_SEARCH_SYSTEM32 |
                                         LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR |
                                         LOAD_LIBRARY_SEARCH_USER_DIRS);
+
+                /*
+                 * The MIT Kerberos DLL suffers an internal segfault
+                 * for some reason if you unload and reload one within
+                 * the same process. So, make sure that after we load
+                 * this library, we never free it.
+                 *
+                 * Or rather: after we've loaded it once, if any
+                 * _further_ load returns the same module handle, we
+                 * immediately free it again (to prevent the Windows
+                 * API's internal reference count growing without
+                 * bound). But on the other hand we never free it in
+                 * ssh_gss_cleanup.
+                 */
+                if (library_is_in_never_unload_tree(module))
+                    FreeLibrary(module);
+                add_library_to_never_unload_tree(module);
             }
             sfree(buffer);
         }
@@ -280,7 +319,11 @@ void ssh_gss_cleanup(struct ssh_gss_liblist *list)
      * another SSH instance still using it.
      */
     for (i = 0; i < list->nlibraries; i++) {
-        FreeLibrary((HMODULE)list->libraries[i].handle);
+        if (list->libraries[i].id != 0) {
+            HMODULE module = (HMODULE)list->libraries[i].handle;
+            if (!library_is_in_never_unload_tree(module))
+                FreeLibrary(module);
+        }
         if (list->libraries[i].id == 2) {
             /* The 'custom' id involves a dynamically allocated message.
              * Note that we must cast away the 'const' to free it. */