From 4b5f719906f382a3318981126db19d775041c50a Mon Sep 17 00:00:00 2001
From: Eduardo Arias <eduar_hte@hotmail.com>
Date: Mon, 5 Aug 2024 08:49:06 -0700
Subject: [PATCH] Fixed shared files deadlock in a multi-threaded Windows
 application - The shared files Windows implementation introduced in PR #3132
 works   in multi-process single-threaded contexts but it doesn't work  
 correctly in single-process multi-threaded contexts. - The issue is that the
 LockFileEx Win32 function works on a per-handle   basis.   - In a
 multi-process context, each process will have called    
 SharedFiles::add_new_handler when initializing the SharedFile and    
 obtained a handle, and thus locking will work.   - When running ModSecurity
 in a single process using multiple threads,     the initialization of the
 SharedFile will happen once and the handle     will be shared by all threads.
 Then, if two threads try to write to     the same shared file concurrently,
 they may deadlock as one of them     will lock the file (by calling
 LockFileEx) and then proceed to write     to the file. If before writing to
 the file and unlocking it, another     thread calls LockFileEx on the same
 handle, the attempt to write to     the file will lock generating a deadlock.
 - The new implementation replaces usage of LockFileEx/UnlockFileEx with   a
 named mutex to lock access to the shared file.   - A named mutex is used to
 support multi-process scenarios.   - The mutex name is generated using the
 filename to support multiple     shared files (such as that for the debug and
 audit logs).     - This assumes that both process will initialize the
 SharedFile       instance using the same filename (which is expected as
 they'd be       using the same configuration file)

---
 src/utils/shared_files.cc | 44 ++++++++++++++++++++++++++++++++-------
 src/utils/shared_files.h  |  6 ++++++
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/src/utils/shared_files.cc b/src/utils/shared_files.cc
index df18022f7c..6982d0d6e9 100644
--- a/src/utils/shared_files.cc
+++ b/src/utils/shared_files.cc
@@ -17,8 +17,7 @@
 
 #include <fcntl.h>
 #ifdef WIN32
-#include <windows.h>
-#include <io.h>
+#include <algorithm>
 #endif
 
 
@@ -34,7 +33,32 @@ SharedFiles::handlers_map::iterator SharedFiles::add_new_handler(
         return m_handlers.end();
     }
 
-    return m_handlers.insert({ fileName, {fp, 0} }).first;
+#ifdef WIN32
+    // replace invalid characters for a Win32 named object
+    auto tmp = fileName;
+    std::replace(tmp.begin(), tmp.end(), '\\', '_');
+    std::replace(tmp.begin(), tmp.end(), '/', '_');
+
+    // use named mutex for multi-process locking support
+    const auto mutexName = "Global\\ModSecurity_" + tmp;
+
+    HANDLE hMutex = CreateMutex(NULL, FALSE, mutexName.c_str());
+    if (hMutex == NULL) {
+        error->assign("Failed to create mutex for shared file: " + fileName);
+        fclose(fp);
+        return m_handlers.end();
+    }
+#endif
+
+    auto handler = handler_info {
+        fp,
+#ifdef WIN32
+        hMutex,
+#endif
+        0
+    };
+    // cppcheck-suppress resourceLeak ; false positive, fp is closed in SharedFiles::close
+    return m_handlers.insert({ fileName, handler }).first;
 }
 
 
@@ -69,6 +93,9 @@ void SharedFiles::close(const std::string& fileName) {
     if (it->second.cnt == 0)
     {
         fclose(it->second.fp);
+#ifdef WIN32
+        CloseHandle(it->second.hMutex);
+#endif
 
         m_handlers.erase(it);
     }
@@ -92,9 +119,11 @@ bool SharedFiles::write(const std::string& fileName,
     lock.l_type = F_WRLCK;
     fcntl(fileno(it->second.fp), F_SETLKW, &lock);
 #else
-    auto handle = reinterpret_cast<HANDLE>(_get_osfhandle(fileno(it->second.fp)));
-    OVERLAPPED overlapped = { 0 };
-    ::LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, MAXDWORD, MAXDWORD, &overlapped);
+    DWORD dwWaitResult = WaitForSingleObject(it->second.hMutex, INFINITE);
+    if (dwWaitResult != WAIT_OBJECT_0) {
+        error->assign("couldn't lock shared file: " + fileName);
+        return false;
+    }
 #endif
 
     auto wrote = fwrite(msg.c_str(), 1, msg.size(), it->second.fp);
@@ -109,8 +138,7 @@ bool SharedFiles::write(const std::string& fileName,
     lock.l_type = F_UNLCK;
     fcntl(fileno(it->second.fp), F_SETLKW, &lock);
 #else
-    overlapped = { 0 };
-    ::UnlockFileEx(handle, 0, MAXDWORD, MAXDWORD, &overlapped);
+    ::ReleaseMutex(it->second.hMutex);
 #endif
 
     return ret;
diff --git a/src/utils/shared_files.h b/src/utils/shared_files.h
index 4953eeff4a..fcc78c9863 100644
--- a/src/utils/shared_files.h
+++ b/src/utils/shared_files.h
@@ -18,6 +18,9 @@
 
 
 #include <stdio.h>
+#ifdef WIN32
+#include <Windows.h>
+#endif
 
 #include <unordered_map>
 #include <string>
@@ -53,6 +56,9 @@ class SharedFiles {
 
      struct handler_info {
          FILE* fp;
+#ifdef WIN32
+         HANDLE hMutex;
+#endif
          unsigned int cnt;
      };