x64dbg
/
TitanEngine
mirror of https://github.com/x64dbg/TitanEngine
1
0
Fork 0
Code Releases Activity

Merge pull request #35 from x64dbg/membp-race 

Fix race condition when large-region memory breakpoints
This commit is contained in:
Duncan Ogilvie 2026-07-16 14:58:19 +02:00 committed by GitHub
parent 7acc78afd1 0bb4e1ab9b
commit 810ab65f10
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 17 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
TitanEngine/TitanEngine.Debugger.DebugLoop.cpp
View File

@ -957,6 +957,11 @@ __declspec(dllexport) void TITCALL DebugLoop()
case STATUS_GUARD_PAGE_VIOLATION: case STATUS_GUARD_PAGE_VIOLATION:
case STATUS_ACCESS_VIOLATION: case STATUS_ACCESS_VIOLATION:
{ {
// SetMemoryBPXEx changes page protections before publishing the
// matching metadata. Wait for that transaction before inspecting the
// memory-breakpoint containers.
CriticalSectionLocker breakpointLock(LockBreakPointBuffer);
// Plan (making sure the breakpoint is valid): // Plan (making sure the breakpoint is valid):
// 1) Check if one of our BPs falls into the access address // 1) Check if one of our BPs falls into the access address
// 2) Check if this breakpoint is of the right type (READ, WRITE, etc) // 2) Check if this breakpoint is of the right type (READ, WRITE, etc)
@ -1006,6 +1011,11 @@ __declspec(dllexport) void TITCALL DebugLoop()
} }
auto hitPage = MemoryBreakpointPages.find(currentPageAddr); auto hitPage = MemoryBreakpointPages.find(currentPageAddr);
const bool hasHitPage = hitPage != MemoryBreakpointPages.end();
DWORD originalProtect = 0;
if(hasHitPage)
originalProtect = hitPage->second.origProtect;
if(!bFoundBreakPoint) if(!bFoundBreakPoint)
{ {
// There were no BPs at the accessed address. // There were no BPs at the accessed address.
@ -1111,8 +1121,7 @@ __declspec(dllexport) void TITCALL DebugLoop()
// an infinite loop (single-stepping might fail and call this handler, which will try to automatically // an infinite loop (single-stepping might fail and call this handler, which will try to automatically
// single-step again and end up at this exact place, and so on). So if we are sure that resetting the BP is not a good idea, // single-step again and end up at this exact place, and so on). So if we are sure that resetting the BP is not a good idea,
// we just pass the exception on. Or maybe it's better to set PAGE_EXECUTE_READWRITE and simply continue? // we just pass the exception on. Or maybe it's better to set PAGE_EXECUTE_READWRITE and simply continue?
DWORD originalProtect = hitPage->second.origProtect; if(hasHitPage && ResetMemBPX && (bCallUserCallback || IsMemoryAccessAllowed(originalProtect, accessType)))
if(ResetMemBPX && (bCallUserCallback || IsMemoryAccessAllowed(originalProtect, accessType)))
{ {
// Mini Plan: // Mini Plan:
// 1) Set a protection option that would allow us to normally execute the instruction that caused this exception // 1) Set a protection option that would allow us to normally execute the instruction that caused this exception
@ -1123,12 +1132,18 @@ __declspec(dllexport) void TITCALL DebugLoop()
if(bCallUserCallback) if(bCallUserCallback)
{ {
// The callback can synchronously add or remove breakpoints.
// All metadata needed below was copied while locked, so release
// the transaction lock before entering x64dbg.
breakpointLock.unlock();
myCustomHandler = (fCustomHandler)(foundBreakPoint.ExecuteCallBack); myCustomHandler = (fCustomHandler)(foundBreakPoint.ExecuteCallBack);
myCustomHandler((void*)accessAddr); myCustomHandler((void*)accessAddr);
} }
ResetMemBpxCallback = [currentPageAddr] ResetMemBpxCallback = [currentPageAddr]
{ {
CriticalSectionLocker breakpointLock(LockBreakPointBuffer);
// We have successfully executed the instruction! // We have successfully executed the instruction!
// But by this point the breakpoint could have been removed in a callback. // But by this point the breakpoint could have been removed in a callback.
// We should check if it's still here (or some of our other breakpoints), // We should check if it's still here (or some of our other breakpoints),