From 4539a9b25437a0c2ea04db308804d028651b47f1 Mon Sep 17 00:00:00 2001 From: Michael Maltsev <4129781+m417z@users.noreply.github.com> Date: Sun, 6 Apr 2025 23:13:37 +0300 Subject: [PATCH 1/3] Free unused trampoline regions on abort and attach error Similar to the implementation in SlimDetoursTransactionCommit. --- Source/SlimDetours/SlimDetours.inl | 4 +++ Source/SlimDetours/Trampoline.c | 47 +++++++++++++++++++++++++----- Source/SlimDetours/Transaction.c | 7 +++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/Source/SlimDetours/SlimDetours.inl b/Source/SlimDetours/SlimDetours.inl index 8830da4b..5a34dc1d 100644 --- a/Source/SlimDetours/SlimDetours.inl +++ b/Source/SlimDetours/SlimDetours.inl @@ -269,6 +269,10 @@ detour_free_trampoline( VOID detour_free_unused_trampoline_regions(VOID); +VOID +detour_free_trampoline_region_if_unused( + _In_ PDETOUR_TRAMPOLINE pTrampoline); + BYTE detour_align_from_trampoline( _In_ PDETOUR_TRAMPOLINE pTrampoline, diff --git a/Source/SlimDetours/Trampoline.c b/Source/SlimDetours/Trampoline.c index ed38570b..05c63500 100644 --- a/Source/SlimDetours/Trampoline.c +++ b/Source/SlimDetours/Trampoline.c @@ -400,12 +400,21 @@ detour_is_region_empty( return TRUE; } +static VOID -detour_free_unused_trampoline_regions(VOID) +detour_free_region( + _In_ PDETOUR_REGION* ppRegionBase, + _In_ PDETOUR_REGION pRegion) { - PVOID pMem; - SIZE_T sMem; + *ppRegionBase = pRegion->pNext; + PVOID pMem = pRegion; + SIZE_T sMem = 0; + NtFreeVirtualMemory(NtCurrentProcess(), &pMem, &sMem, MEM_RELEASE); +} +VOID +detour_free_unused_trampoline_regions(VOID) +{ PDETOUR_REGION* ppRegionBase = &s_pRegions; PDETOUR_REGION pRegion = s_pRegions; @@ -413,11 +422,7 @@ detour_free_unused_trampoline_regions(VOID) { if (detour_is_region_empty(pRegion)) { - *ppRegionBase = pRegion->pNext; - - pMem = pRegion; - sMem = 0; - NtFreeVirtualMemory(NtCurrentProcess(), &pMem, &sMem, MEM_RELEASE); + detour_free_region(ppRegionBase, pRegion); s_pRegion = NULL; } else { @@ -427,6 +432,32 @@ detour_free_unused_trampoline_regions(VOID) } } +VOID +detour_free_trampoline_region_if_unused( + _In_ PDETOUR_TRAMPOLINE pTrampoline) +{ + PDETOUR_REGION pTargetRegion = (PDETOUR_REGION)((ULONG_PTR)pTrampoline & ~(ULONG_PTR)0xffff); + + PDETOUR_REGION* ppRegionBase = &s_pRegions; + PDETOUR_REGION pRegion = s_pRegions; + + while (pRegion != NULL) + { + if (pRegion == pTargetRegion) + { + if (detour_is_region_empty(pRegion)) + { + detour_free_region(ppRegionBase, pRegion); + s_pRegion = NULL; + } + break; + } + + ppRegionBase = &pRegion->pNext; + pRegion = *ppRegionBase; + } +} + BYTE detour_align_from_trampoline( _In_ PDETOUR_TRAMPOLINE pTrampoline, diff --git a/Source/SlimDetours/Transaction.c b/Source/SlimDetours/Transaction.c index 66ca0e12..953e1562 100644 --- a/Source/SlimDetours/Transaction.c +++ b/Source/SlimDetours/Transaction.c @@ -106,6 +106,7 @@ SlimDetoursTransactionAbort(VOID) PVOID pMem; SIZE_T sMem; DWORD dwOld; + BOOL freed = FALSE; if (s_nPendingThreadId != NtCurrentThreadId()) { @@ -123,6 +124,7 @@ SlimDetoursTransactionAbort(VOID) { detour_free_trampoline(o->pTrampoline); o->pTrampoline = NULL; + freed = TRUE; } PDETOUR_OPERATION n = o->pNext; @@ -130,6 +132,10 @@ SlimDetoursTransactionAbort(VOID) o = n; } s_pPendingOperations = NULL; + if (freed) + { + detour_free_unused_trampoline_regions(); + } // Make sure the trampoline pages are no longer writable. detour_runnable_trampoline_regions(); @@ -333,6 +339,7 @@ SlimDetoursAttach( if (pTrampoline != NULL) { detour_free_trampoline(pTrampoline); + detour_free_trampoline_region_if_unused(pTrampoline); pTrampoline = NULL; } if (o != NULL) From 06bcb221cdf47b4e962d7b58df15dbcaf93aac44 Mon Sep 17 00:00:00 2001 From: Michael Maltsev <4129781+m417z@users.noreply.github.com> Date: Thu, 10 Apr 2025 22:04:09 +0300 Subject: [PATCH 2/3] Refactor DETOUR_OPERATION struct --- Source/SlimDetours/SlimDetours.inl | 10 ++++++++-- Source/SlimDetours/Thread.c | 4 ++-- Source/SlimDetours/Transaction.c | 16 +++++++--------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Source/SlimDetours/SlimDetours.inl b/Source/SlimDetours/SlimDetours.inl index 5a34dc1d..f8fd0308 100644 --- a/Source/SlimDetours/SlimDetours.inl +++ b/Source/SlimDetours/SlimDetours.inl @@ -100,13 +100,19 @@ _STATIC_ASSERT(sizeof(DETOUR_TRAMPOLINE) == 96); _STATIC_ASSERT(sizeof(DETOUR_TRAMPOLINE) == 184); #endif +enum +{ + DETOUR_OPERATION_NONE = 0, + DETOUR_OPERATION_ADD, + DETOUR_OPERATION_REMOVE, +}; + typedef struct _DETOUR_OPERATION DETOUR_OPERATION, *PDETOUR_OPERATION; struct _DETOUR_OPERATION { PDETOUR_OPERATION pNext; - BOOL fIsAdd : 1; - BOOL fIsRemove : 1; + DWORD dwOperation; PBYTE* ppbPointer; PBYTE pbTarget; PDETOUR_TRAMPOLINE pTrampoline; diff --git a/Source/SlimDetours/Thread.c b/Source/SlimDetours/Thread.c index 309f9590..688eaf51 100644 --- a/Source/SlimDetours/Thread.c +++ b/Source/SlimDetours/Thread.c @@ -188,7 +188,7 @@ detour_thread_update( bUpdateContext = FALSE; for (o = PendingOperations; o != NULL && !bUpdateContext; o = o->pNext) { - if (o->fIsRemove) + if (o->dwOperation == DETOUR_OPERATION_REMOVE) { if (cxt.CONTEXT_PC >= (ULONG_PTR)o->pTrampoline->rbCode && cxt.CONTEXT_PC < ((ULONG_PTR)o->pTrampoline->rbCode + RTL_FIELD_SIZE(DETOUR_TRAMPOLINE, rbCode))) @@ -204,7 +204,7 @@ detour_thread_update( bUpdateContext = TRUE; } #endif - } else if (o->fIsAdd) + } else if (o->dwOperation == DETOUR_OPERATION_ADD) { if (cxt.CONTEXT_PC >= (ULONG_PTR)o->pbTarget && cxt.CONTEXT_PC < ((ULONG_PTR)o->pbTarget + o->pTrampoline->cbRestore)) diff --git a/Source/SlimDetours/Transaction.c b/Source/SlimDetours/Transaction.c index 953e1562..eebe983e 100644 --- a/Source/SlimDetours/Transaction.c +++ b/Source/SlimDetours/Transaction.c @@ -120,7 +120,7 @@ SlimDetoursTransactionAbort(VOID) pMem = o->pbTarget; sMem = o->pTrampoline->cbRestore; NtProtectVirtualMemory(NtCurrentProcess(), &pMem, &sMem, o->dwPerm, &dwOld); - if (o->fIsAdd) + if (o->dwOperation == DETOUR_OPERATION_ADD) { detour_free_trampoline(o->pTrampoline); o->pTrampoline = NULL; @@ -177,7 +177,7 @@ SlimDetoursTransactionCommit(VOID) o = s_pPendingOperations; do { - if (o->fIsRemove) + if (o->dwOperation == DETOUR_OPERATION_REMOVE) { // Check if the jmps still points where we expect, otherwise someone might have hooked us. BOOL hookIsStillThere = @@ -195,13 +195,13 @@ SlimDetoursTransactionCommit(VOID) } else { // Don't remove in this case, put in bypass mode and leak trampoline. - o->fIsRemove = FALSE; + o->dwOperation = DETOUR_OPERATION_NONE; o->pTrampoline->pbDetour = o->pTrampoline->rbCode; DETOUR_TRACE("detours: Leaked hook on pbTarget=%p due to external hooking\n", o->pbTarget); } *o->ppbPointer = o->pbTarget; - } else if (o->fIsAdd) + } else if (o->dwOperation == DETOUR_OPERATION_ADD) { DETOUR_TRACE("detours: pbTramp =%p, pbRemain=%p, pbDetour=%p, cbRestore=%u\n", o->pTrampoline, @@ -268,7 +268,7 @@ SlimDetoursTransactionCommit(VOID) pMem = o->pbTarget; sMem = o->pTrampoline->cbRestore; NtProtectVirtualMemory(NtCurrentProcess(), &pMem, &sMem, o->dwPerm, &dwOld); - if (o->fIsRemove) + if (o->dwOperation == DETOUR_OPERATION_REMOVE) { detour_free_trampoline(o->pTrampoline); o->pTrampoline = NULL; @@ -492,8 +492,7 @@ SlimDetoursAttach( pTrampoline->rbCode[8], pTrampoline->rbCode[9], pTrampoline->rbCode[10], pTrampoline->rbCode[11]); - o->fIsAdd = TRUE; - o->fIsRemove = FALSE; + o->dwOperation = DETOUR_OPERATION_ADD; o->ppbPointer = (PBYTE*)ppPointer; o->pTrampoline = pTrampoline; o->pbTarget = pbTarget; @@ -556,8 +555,7 @@ SlimDetoursDetach( goto fail; } - o->fIsAdd = FALSE; - o->fIsRemove = TRUE; + o->dwOperation = DETOUR_OPERATION_REMOVE; o->ppbPointer = (PBYTE*)ppPointer; o->pTrampoline = pTrampoline; o->pbTarget = pbTarget; From d425fa4f49b2549bc64a274382d0d4eaded846de Mon Sep 17 00:00:00 2001 From: Michael Maltsev <4129781+m417z@users.noreply.github.com> Date: Fri, 11 Apr 2025 17:09:08 +0300 Subject: [PATCH 3/3] Add SlimDetoursDetachEx to allow to free the trampoline manually Usage example: SlimDetoursTransactionBegin(); PVOID pTrampoline = NULL; DETOUR_DETACH_OPTIONS Options; Options.ppTrampolineToFreeManually = &pTrampoline; SlimDetoursDetachEx(pPointer, pDetour, &Options); SlimDetoursTransactionCommit(); Sleep(1000); SlimDetoursFreeTrampoline(pTrampoline); --- Source/SlimDetours/SlimDetours.h | 26 +++++++++- Source/SlimDetours/SlimDetours.inl | 1 + Source/SlimDetours/Transaction.c | 77 +++++++++++++++++++++++++++--- 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/Source/SlimDetours/SlimDetours.h b/Source/SlimDetours/SlimDetours.h index 8109cbc7..ee2d12a7 100644 --- a/Source/SlimDetours/SlimDetours.h +++ b/Source/SlimDetours/SlimDetours.h @@ -62,11 +62,35 @@ SlimDetoursAttach( _Inout_ PVOID* ppPointer, _In_ PVOID pDetour); +typedef struct _DETOUR_DETACH_OPTIONS +{ + PVOID *ppTrampolineToFreeManually; +} DETOUR_DETACH_OPTIONS, *PDETOUR_DETACH_OPTIONS; + +typedef const DETOUR_DETACH_OPTIONS* PCDETOUR_DETACH_OPTIONS; + HRESULT NTAPI +SlimDetoursDetachEx( + _Inout_ PVOID* ppPointer, + _In_ PVOID pDetour, + _In_ PCDETOUR_DETACH_OPTIONS pOptions); + +FORCEINLINE +HRESULT SlimDetoursDetach( _Inout_ PVOID* ppPointer, - _In_ PVOID pDetour); + _In_ PVOID pDetour) +{ + DETOUR_DETACH_OPTIONS Options; + Options.ppTrampolineToFreeManually = NULL; + return SlimDetoursDetachEx(ppPointer, pDetour, &Options); +} + +HRESULT +NTAPI +SlimDetoursFreeTrampoline( + _In_ PVOID pTrampoline); PVOID NTAPI diff --git a/Source/SlimDetours/SlimDetours.inl b/Source/SlimDetours/SlimDetours.inl index f8fd0308..f3121421 100644 --- a/Source/SlimDetours/SlimDetours.inl +++ b/Source/SlimDetours/SlimDetours.inl @@ -117,6 +117,7 @@ struct _DETOUR_OPERATION PBYTE pbTarget; PDETOUR_TRAMPOLINE pTrampoline; ULONG dwPerm; + PVOID* ppTrampolineToFreeManually; }; /* Memory management */ diff --git a/Source/SlimDetours/Transaction.c b/Source/SlimDetours/Transaction.c index eebe983e..496878e9 100644 --- a/Source/SlimDetours/Transaction.c +++ b/Source/SlimDetours/Transaction.c @@ -194,12 +194,14 @@ SlimDetoursTransactionCommit(VOID) NtFlushInstructionCache(NtCurrentProcess(), o->pbTarget, o->pTrampoline->cbRestore); } else { - // Don't remove in this case, put in bypass mode and leak trampoline. + // Don't remove and leak trampoline in this case. o->dwOperation = DETOUR_OPERATION_NONE; - o->pTrampoline->pbDetour = o->pTrampoline->rbCode; DETOUR_TRACE("detours: Leaked hook on pbTarget=%p due to external hooking\n", o->pbTarget); } + // Put hook in bypass mode. + o->pTrampoline->pbDetour = o->pTrampoline->rbCode; + *o->ppbPointer = o->pbTarget; } else if (o->dwOperation == DETOUR_OPERATION_ADD) { @@ -270,9 +272,16 @@ SlimDetoursTransactionCommit(VOID) NtProtectVirtualMemory(NtCurrentProcess(), &pMem, &sMem, o->dwPerm, &dwOld); if (o->dwOperation == DETOUR_OPERATION_REMOVE) { - detour_free_trampoline(o->pTrampoline); + if (!o->ppTrampolineToFreeManually) + { + detour_free_trampoline(o->pTrampoline); + freed = TRUE; + } else + { + // The caller is responsible for freeing the trampoline. + *o->ppTrampolineToFreeManually = o->pTrampoline; + } o->pTrampoline = NULL; - freed = TRUE; } n = o->pNext; @@ -505,9 +514,10 @@ SlimDetoursAttach( HRESULT NTAPI -SlimDetoursDetach( +SlimDetoursDetachEx( _Inout_ PVOID* ppPointer, - _In_ PVOID pDetour) + _In_ PVOID pDetour, + _In_ PCDETOUR_DETACH_OPTIONS pOptions) { NTSTATUS Status; PVOID pMem; @@ -560,12 +570,67 @@ SlimDetoursDetach( o->pTrampoline = pTrampoline; o->pbTarget = pbTarget; o->dwPerm = dwOld; + o->ppTrampolineToFreeManually = pOptions->ppTrampolineToFreeManually; o->pNext = s_pPendingOperations; s_pPendingOperations = o; return HRESULT_FROM_NT(STATUS_SUCCESS); } +HRESULT +NTAPI +SlimDetoursFreeTrampoline( + _In_ PVOID pTrampoline) +{ + if (pTrampoline == NULL) + { + return HRESULT_FROM_NT(STATUS_SUCCESS); + } + + // This function can be called as part of a transaction or outside of a transaction. + ULONG nPrevPendingThreadId = _InterlockedCompareExchange(&s_nPendingThreadId, NtCurrentThreadId(), 0); + BOOL bInTransaction = nPrevPendingThreadId != 0; + if (bInTransaction && nPrevPendingThreadId != NtCurrentThreadId()) + { + return HRESULT_FROM_NT(STATUS_TRANSACTIONAL_CONFLICT); + } + + NTSTATUS Status; + + if (!bInTransaction) + { + // Make sure the trampoline pages are writable. + Status = detour_writable_trampoline_regions(); + if (!NT_SUCCESS(Status)) + { + goto fail; + } + } + + detour_free_trampoline((PDETOUR_TRAMPOLINE)pTrampoline); + detour_free_trampoline_region_if_unused((PDETOUR_TRAMPOLINE)pTrampoline); + + if (!bInTransaction) + { + detour_runnable_trampoline_regions(); + } + + Status = STATUS_SUCCESS; + +fail: + if (!bInTransaction) + { +#ifdef _MSC_VER +#pragma warning(disable: __WARNING_INTERLOCKED_ACCESS) +#endif + s_nPendingThreadId = 0; +#ifdef _MSC_VER +#pragma warning(default: __WARNING_INTERLOCKED_ACCESS) +#endif + } + return HRESULT_FROM_NT(Status); +} + HRESULT NTAPI SlimDetoursUninitialize(VOID)