From cdb1af48e0532961af784856a1a1707e30658454 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Fri, 16 Oct 2026 18:14:21 +0900 Subject: [PATCH] Windows: clean up partial encryption pool startup Join created workers and release allocated handles when startup fails before the pool is published. Close worker handles on normal shutdown as well. Test each startup allocation failure and subsequent restart. --- Tests/test_windows_kdf_completion.py | 2 + Tests/windows_kdf_completion_test.c | 115 +++++++++++++++++++++++++++ src/Common/EncryptionThreadPool.c | 55 +++++++++---- 3 files changed, 155 insertions(+), 17 deletions(-) diff --git a/Tests/test_windows_kdf_completion.py b/Tests/test_windows_kdf_completion.py index c5163adf..bb8daa76 100644 --- a/Tests/test_windows_kdf_completion.py +++ b/Tests/test_windows_kdf_completion.py @@ -8,6 +8,7 @@ The harness compiles the production thread pool with event-controlled KDF stubs. Initialization and draining are extracted from ReadVolumeHeaderWithAbort, so the caller and worker reference protocol are checked together. All request storage is retained until pool shutdown joins the workers. +Startup checks fail each resource creation in turn and verify cleanup and restart. """ import argparse import os @@ -68,6 +69,7 @@ def main(): raise RuntimeError(result.stdout + result.stderr) for workers in (2, 8): subprocess.run([str(exe), str(workers)], cwd=build, check=True, timeout=90) + subprocess.run([str(exe), "startup"], cwd=build, check=True, timeout=90) if __name__ == "__main__": diff --git a/Tests/windows_kdf_completion_test.c b/Tests/windows_kdf_completion_test.c index 7cabe6cc..6a993b58 100644 --- a/Tests/windows_kdf_completion_test.c +++ b/Tests/windows_kdf_completion_test.c @@ -4,14 +4,27 @@ */ #include "Common/EncryptionThreadPool.h" #include "Common/Pkcs5.h" +#include #include static BOOL TestSetEvent (HANDLE event); static DWORD TestWait (HANDLE event); +static HANDLE WINAPI TestCreateEvent (LPSECURITY_ATTRIBUTES attributes, BOOL manualReset, BOOL initialState, LPCWSTR name); +static HANDLE WINAPI TestCreateMutex (LPSECURITY_ATTRIBUTES attributes, BOOL initialOwner, LPCWSTR name); +static uintptr_t TestBeginThread (void *security, unsigned stackSize, unsigned (__stdcall *proc) (void *), void *arg, unsigned flags, unsigned *id); +static BOOL WINAPI TestCloseHandle (HANDLE handle); #undef TC_WAIT_EVENT #define TC_WAIT_EVENT(event) TestWait (event) #define SetEvent TestSetEvent +#define CreateEventW TestCreateEvent +#define CreateMutexW TestCreateMutex +#define _beginthreadex TestBeginThread +#define CloseHandle TestCloseHandle #include "Common/EncryptionThreadPool.c" +#undef CloseHandle +#undef _beginthreadex +#undef CreateMutexW +#undef CreateEventW #undef SetEvent #include "kdf_item.inc" @@ -46,6 +59,14 @@ static int RequestCount; static __declspec(thread) Job *CurrentJob; static __declspec(thread) Request *DrainingRequest; +typedef struct +{ + HANDLE Handle; + BOOL Thread, Open; +} PoolResource; +static PoolResource Resources[1024]; +static int ResourceCount, OpenResources, CreateAttempts, FailCreateAt; + static void Require (BOOL condition, const char *message) { if (!condition) @@ -55,6 +76,66 @@ static void Require (BOOL condition, const char *message) } } +static BOOL FailCreation (void) +{ + if (++CreateAttempts == FailCreateAt) + { + SetLastError (ERROR_NOT_ENOUGH_MEMORY); + return TRUE; + } + return FALSE; +} + +static HANDLE TrackResource (HANDLE handle, BOOL thread) +{ + if (handle) + { + PoolResource *resource; + Require (ResourceCount < sizeof (Resources) / sizeof (Resources[0]), "resource fixture capacity"); + resource = &Resources[ResourceCount++]; + resource->Handle = handle; + resource->Thread = thread; + resource->Open = TRUE; + ++OpenResources; + } + return handle; +} + +static HANDLE WINAPI TestCreateEvent (LPSECURITY_ATTRIBUTES attributes, BOOL manualReset, BOOL initialState, LPCWSTR name) +{ + return FailCreation () ? NULL : TrackResource (CreateEventW (attributes, manualReset, initialState, name), FALSE); +} + +static HANDLE WINAPI TestCreateMutex (LPSECURITY_ATTRIBUTES attributes, BOOL initialOwner, LPCWSTR name) +{ + return FailCreation () ? NULL : TrackResource (CreateMutexW (attributes, initialOwner, name), FALSE); +} + +static uintptr_t TestBeginThread (void *security, unsigned stackSize, unsigned (__stdcall *proc) (void *), void *arg, unsigned flags, unsigned *id) +{ + return FailCreation () ? 0 : (uintptr_t) TrackResource ((HANDLE) _beginthreadex (security, stackSize, proc, arg, flags, id), TRUE); +} + +static BOOL WINAPI TestCloseHandle (HANDLE handle) +{ + int i; + for (i = ResourceCount - 1; i >= 0; --i) + { + PoolResource *resource = &Resources[i]; + if (resource->Open && resource->Handle == handle) + { + if (resource->Thread) + Require (WaitForSingleObject (handle, 0) == WAIT_OBJECT_0, "worker handle closed before join"); + resource->Open = FALSE; + --OpenResources; + Require (CloseHandle (handle), "close pool resource"); + return TRUE; + } + } + Require (FALSE, "closing unknown pool resource"); + return FALSE; +} + static HANDLE Event (void) { HANDLE event = CreateEvent (NULL, FALSE, FALSE, NULL); @@ -285,6 +366,34 @@ static void PublicationBoundaries (void) JobDone (job); } +static void ResetResources (int failurePoint) +{ + Require (OpenResources == 0, "pool resources leaked"); + ResourceCount = CreateAttempts = 0; + FailCreateAt = failurePoint; +} + +static void StartupFailures (size_t available) +{ + int i, creationCount; + Require (EncryptionThreadPoolStart (available - 2), "start baseline pool"); + creationCount = CreateAttempts; + EncryptionThreadPoolStop (); + ResetResources (0); + for (i = 1; i <= creationCount; ++i) + { + ResetResources (i); + Require (!EncryptionThreadPoolStart (available - 2), "injected startup failure accepted"); + Require (!IsEncryptionThreadPoolRunning () && GetEncryptionThreadCount () == 0, "failed pool published"); + EncryptionThreadPoolStop (); + ResetResources (0); + Require (EncryptionThreadPoolStart (available - 2), "restart after failure"); + EncryptionThreadPoolStop (); + } + ResetResources (0); + printf ("PASS: %d startup failure points, each followed by a successful restart\n", creationCount); +} + int main (int argc, char **argv) { WORD groups; @@ -292,6 +401,11 @@ int main (int argc, char **argv) size_t wanted = argc > 1 ? (size_t) atoi (argv[1]) : 2; int i, j; if (available < 2) { puts ("SKIP: at least two logical CPUs required"); return 0; } + if (argc > 1 && strcmp (argv[1], "startup") == 0) + { + StartupFailures (available); + return 0; + } if (wanted > available) wanted = available; Require (EncryptionThreadPoolStart (available - wanted), "start production pool"); Require (GetEncryptionThreadCount () == wanted, "pool worker count"); @@ -303,6 +417,7 @@ int main (int argc, char **argv) /* Join every worker before releasing any request-owned allocation or event. */ EncryptionThreadPoolStop (); + Require (OpenResources == 0, "pool resources leaked"); for (i = 0; i < RequestCount; ++i) { Request *r = &Requests[i]; diff --git a/src/Common/EncryptionThreadPool.c b/src/Common/EncryptionThreadPool.c index 073305db..6abada10 100644 --- a/src/Common/EncryptionThreadPool.c +++ b/src/Common/EncryptionThreadPool.c @@ -131,6 +131,8 @@ static TC_MUTEX DequeueMutex; static TC_EVENT WorkItemReadyEvent; static TC_EVENT WorkItemCompletedEvent; +static void EncryptionThreadPoolCleanup (void); + void EncryptDataUnitsCurrentThreadEx (unsigned __int8 *buf, const UINT64_STRUCT *structUnitNo, TC_LARGEST_COMPILER_UINT nbrUnits, PCRYPTO_INFO ci) { if (IsRamEncryptionEnabled()) @@ -379,11 +381,11 @@ BOOL EncryptionThreadPoolStart (size_t encryptionFreeCpuCount) #else WorkItemReadyEvent = CreateEvent (NULL, FALSE, FALSE, NULL); if (!WorkItemReadyEvent) - return FALSE; + goto err; WorkItemCompletedEvent = CreateEvent (NULL, FALSE, FALSE, NULL); if (!WorkItemCompletedEvent) - return FALSE; + goto err; #endif #ifdef DEVICE_DRIVER @@ -392,11 +394,11 @@ BOOL EncryptionThreadPoolStart (size_t encryptionFreeCpuCount) #else DequeueMutex = CreateMutex (NULL, FALSE, NULL); if (!DequeueMutex) - return FALSE; + goto err; EnqueueMutex = CreateMutex (NULL, FALSE, NULL); if (!EnqueueMutex) - return FALSE; + goto err; #endif memset (WorkItemQueue, 0, sizeof (WorkItemQueue)); @@ -410,10 +412,7 @@ BOOL EncryptionThreadPoolStart (size_t encryptionFreeCpuCount) #else WorkItemQueue[i].ItemCompletedEvent = CreateEvent (NULL, FALSE, FALSE, NULL); if (!WorkItemQueue[i].ItemCompletedEvent) - { - EncryptionThreadPoolStop(); - return FALSE; - } + goto err; #endif } @@ -454,25 +453,36 @@ BOOL EncryptionThreadPoolStart (size_t encryptionFreeCpuCount) #endif { - EncryptionThreadPoolStop(); - return FALSE; + goto err; } } ThreadPoolRunning = TRUE; return TRUE; + +err: + EncryptionThreadPoolCleanup(); + return FALSE; } void EncryptionThreadPoolStop () { - size_t i; - if (!ThreadPoolRunning) return; + EncryptionThreadPoolCleanup(); +} + + +// Also used before ThreadPoolRunning is published if startup fails partway through. +static void EncryptionThreadPoolCleanup (void) +{ + size_t i; + StopPending = TRUE; - TC_SET_EVENT (WorkItemReadyEvent); + if (ThreadCount > 0) + TC_SET_EVENT (WorkItemReadyEvent); for (i = 0; i < ThreadCount; ++i) { @@ -480,22 +490,33 @@ void EncryptionThreadPoolStop () TCStopThread (ThreadHandles[i], &WorkItemReadyEvent); #else TC_WAIT_EVENT (ThreadHandles[i]); + CloseHandle (ThreadHandles[i]); #endif + ThreadHandles[i] = NULL; } ThreadCount = 0; #ifndef DEVICE_DRIVER - CloseHandle (DequeueMutex); - CloseHandle (EnqueueMutex); + if (DequeueMutex) + CloseHandle (DequeueMutex); + DequeueMutex = NULL; + if (EnqueueMutex) + CloseHandle (EnqueueMutex); + EnqueueMutex = NULL; - CloseHandle (WorkItemReadyEvent); - CloseHandle (WorkItemCompletedEvent); + if (WorkItemReadyEvent) + CloseHandle (WorkItemReadyEvent); + WorkItemReadyEvent = NULL; + if (WorkItemCompletedEvent) + CloseHandle (WorkItemCompletedEvent); + WorkItemCompletedEvent = NULL; for (i = 0; i < sizeof (WorkItemQueue) / sizeof (WorkItemQueue[0]); ++i) { if (WorkItemQueue[i].ItemCompletedEvent) CloseHandle (WorkItemQueue[i].ItemCompletedEvent); + WorkItemQueue[i].ItemCompletedEvent = NULL; } #endif