[KERNEL32]: User-mode Debugging fixes.

Bug: RemoveHandles was checking for process AND thread handle match, instead of OR.
Bug: CloseAllProcessHandles and RemoveHandles's processing loop was buggy with its unlinking and re-use of nested ThreadData.
Bug: ProcessIdToHandle was requesting more process rights than needed.
Bug: Some ULONG<->BOOL conversions were dirty (as in, could result in values of 2 or higher).
Bug: In WaitForDebugEvent, During CREATE_PROCESS_DEBUG_EVENT, the wrong handle was saved as the thread handle.
Bug: In WaitForDebugEvent, all events returned TRUE. Instead, only valid events should return TRUE, and non-existing events should return FALSE.
Add some asserts.
Simplify MarkThreadHandle and MarkProcessHandle.
Simplify OutputDebugStringW.

svn path=/trunk/; revision=52813
This commit is contained in:
Alex Ionescu 2011-07-23 18:28:55 +00:00
parent 3459e1cb1d
commit 90e2e1bad0

View file

@ -264,8 +264,7 @@ MarkThreadHandle(IN DWORD dwThreadId)
PDBGSS_THREAD_DATA ThreadData; PDBGSS_THREAD_DATA ThreadData;
/* Loop all thread data events */ /* Loop all thread data events */
ThreadData = DbgSsGetThreadData(); for (ThreadData = DbgSsGetThreadData(); ThreadData; ThreadData = ThreadData->Next)
while (ThreadData)
{ {
/* Check if this one matches */ /* Check if this one matches */
if (ThreadData->ThreadId == dwThreadId) if (ThreadData->ThreadId == dwThreadId)
@ -274,9 +273,6 @@ MarkThreadHandle(IN DWORD dwThreadId)
ThreadData->HandleMarked = TRUE; ThreadData->HandleMarked = TRUE;
break; break;
} }
/* Move to the next one */
ThreadData = ThreadData->Next;
} }
} }
@ -287,23 +283,15 @@ MarkProcessHandle(IN DWORD dwProcessId)
PDBGSS_THREAD_DATA ThreadData; PDBGSS_THREAD_DATA ThreadData;
/* Loop all thread data events */ /* Loop all thread data events */
ThreadData = DbgSsGetThreadData(); for (ThreadData = DbgSsGetThreadData(); ThreadData; ThreadData = ThreadData->Next)
while (ThreadData)
{ {
/* Check if this one matches */ /* Check if this one matches */
if (ThreadData->ProcessId == dwProcessId) if ((ThreadData->ProcessId == dwProcessId) && !(ThreadData->ThreadId))
{ {
/* Make sure the thread ID is empty */ /* Mark the structure and break out */
if (!ThreadData->ThreadId) ThreadData->HandleMarked = TRUE;
{ break;
/* Mark the structure and break out */
ThreadData->HandleMarked = TRUE;
break;
}
} }
/* Move to the next one */
ThreadData = ThreadData->Next;
} }
} }
@ -312,46 +300,31 @@ WINAPI
RemoveHandles(IN DWORD dwProcessId, RemoveHandles(IN DWORD dwProcessId,
IN DWORD dwThreadId) IN DWORD dwThreadId)
{ {
PDBGSS_THREAD_DATA ThreadData; PDBGSS_THREAD_DATA ThreadData, ThisData;
/* Loop all thread data events */ /* Loop all thread data events */
ThreadData = DbgSsGetThreadData(); ThreadData = DbgSsGetThreadData();
while (ThreadData) for (ThisData = ThreadData; ThisData; ThisData = ThisData->Next)
{ {
/* Check if this one matches */ /* Check if this one matches */
if (ThreadData->ProcessId == dwProcessId) if ((ThisData->HandleMarked) &&
((ThisData->ProcessId == dwProcessId) || (ThisData->ThreadId == dwThreadId)))
{ {
/* Make sure the thread ID matches too */ /* Close open handles */
if (ThreadData->ThreadId == dwThreadId) if (ThisData->ThreadHandle) CloseHandle(ThisData->ThreadHandle);
{ if (ThisData->ProcessHandle) CloseHandle(ThisData->ProcessHandle);
/* Check if we have a thread handle */
if (ThreadData->ThreadHandle)
{
/* Close it */
CloseHandle(ThreadData->ThreadHandle);
}
/* Check if we have a process handle */ /* Unlink the thread data */
if (ThreadData->ProcessHandle) ThreadData->Next = ThisData->Next;
{
/* Close it */
CloseHandle(ThreadData->ProcessHandle);
}
/* Unlink the thread data */ /* Free it*/
DbgSsSetThreadData(ThreadData->Next); RtlFreeHeap(RtlGetProcessHeap(), 0, ThisData);
}
/* Free it*/ else
RtlFreeHeap(RtlGetProcessHeap(), 0, ThreadData); {
/* Move to the next one */
/* Move to the next structure */ ThreadData = ThisData;
ThreadData = DbgSsGetThreadData();
continue;
}
} }
/* Move to the next one */
ThreadData = ThreadData->Next;
} }
} }
@ -359,42 +332,30 @@ VOID
WINAPI WINAPI
CloseAllProcessHandles(IN DWORD dwProcessId) CloseAllProcessHandles(IN DWORD dwProcessId)
{ {
PDBGSS_THREAD_DATA ThreadData; PDBGSS_THREAD_DATA ThreadData, ThisData;
/* Loop all thread data events */ /* Loop all thread data events */
ThreadData = DbgSsGetThreadData(); ThreadData = DbgSsGetThreadData();
while (ThreadData) for (ThisData = ThreadData; ThisData; ThisData = ThisData->Next)
{ {
/* Check if this one matches */ /* Check if this one matches */
if (ThreadData->ProcessId == dwProcessId) if (ThisData->ProcessId == dwProcessId)
{ {
/* Check if we have a thread handle */ /* Close open handles */
if (ThreadData->ThreadHandle) if (ThisData->ThreadHandle) CloseHandle(ThisData->ThreadHandle);
{ if (ThisData->ProcessHandle) CloseHandle(ThisData->ProcessHandle);
/* Close it */
CloseHandle(ThreadData->ThreadHandle);
}
/* Check if we have a process handle */
if (ThreadData->ProcessHandle)
{
/* Close it */
CloseHandle(ThreadData->ProcessHandle);
}
/* Unlink the thread data */ /* Unlink the thread data */
DbgSsSetThreadData(ThreadData->Next); ThreadData->Next = ThisData->Next;
/* Free it*/ /* Free it*/
RtlFreeHeap(RtlGetProcessHeap(), 0, ThreadData); RtlFreeHeap(RtlGetProcessHeap(), 0, ThisData);
}
/* Move to the next structure */ else
ThreadData = DbgSsGetThreadData(); {
continue; /* Move to the next one */
ThreadData = ThisData;
} }
/* Move to the next one */
ThreadData = ThreadData->Next;
} }
} }
@ -415,7 +376,9 @@ ProcessIdToHandle(IN DWORD dwProcessId)
ClientId.UniqueProcess = UlongToHandle(dwProcessId); ClientId.UniqueProcess = UlongToHandle(dwProcessId);
InitializeObjectAttributes(&ObjectAttributes, NULL, 0, NULL, NULL); InitializeObjectAttributes(&ObjectAttributes, NULL, 0, NULL, NULL);
Status = NtOpenProcess(&Handle, Status = NtOpenProcess(&Handle,
PROCESS_ALL_ACCESS, PROCESS_CREATE_THREAD | PROCESS_VM_OPERATION |
PROCESS_VM_WRITE | PROCESS_VM_READ |
PROCESS_SUSPEND_RESUME | PROCESS_QUERY_INFORMATION,
&ObjectAttributes, &ObjectAttributes,
&ClientId); &ClientId);
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
@ -459,7 +422,7 @@ CheckRemoteDebuggerPresent(IN HANDLE hProcess,
if (NT_SUCCESS(Status)) if (NT_SUCCESS(Status))
{ {
/* Return the current state */ /* Return the current state */
*pbDebuggerPresent = (DebugPort) ? TRUE : FALSE; *pbDebuggerPresent = DebugPort != NULL;
return TRUE; return TRUE;
} }
@ -507,7 +470,7 @@ BOOL
WINAPI WINAPI
DebugActiveProcess(IN DWORD dwProcessId) DebugActiveProcess(IN DWORD dwProcessId)
{ {
NTSTATUS Status; NTSTATUS Status, Status1;
HANDLE Handle; HANDLE Handle;
/* Connect to the debugger */ /* Connect to the debugger */
@ -524,7 +487,10 @@ DebugActiveProcess(IN DWORD dwProcessId)
/* Now debug the process */ /* Now debug the process */
Status = DbgUiDebugActiveProcess(Handle); Status = DbgUiDebugActiveProcess(Handle);
NtClose(Handle);
/* Close the handle since we're done */
Status1 = NtClose(Handle);
ASSERT(NT_SUCCESS(Status1));
/* Check if debugging worked */ /* Check if debugging worked */
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
@ -545,7 +511,7 @@ BOOL
WINAPI WINAPI
DebugActiveProcessStop(IN DWORD dwProcessId) DebugActiveProcessStop(IN DWORD dwProcessId)
{ {
NTSTATUS Status; NTSTATUS Status, Status1;
HANDLE Handle; HANDLE Handle;
/* Get the process handle */ /* Get the process handle */
@ -557,7 +523,8 @@ DebugActiveProcessStop(IN DWORD dwProcessId)
/* Now stop debgging the process */ /* Now stop debgging the process */
Status = DbgUiStopDebugging(Handle); Status = DbgUiStopDebugging(Handle);
NtClose(Handle); Status1 = NtClose(Handle);
ASSERT(NT_SUCCESS(Status1));
/* Check for failure */ /* Check for failure */
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
@ -614,7 +581,7 @@ DebugSetProcessKillOnExit(IN BOOL KillOnExit)
} }
/* Now set the kill-on-exit state */ /* Now set the kill-on-exit state */
State = KillOnExit; State = KillOnExit != 0;
Status = NtSetInformationDebugObject(Handle, Status = NtSetInformationDebugObject(Handle,
DebugObjectKillProcessOnExitInformation, DebugObjectKillProcessOnExitInformation,
&State, &State,
@ -711,15 +678,14 @@ WaitForDebugEvent(IN LPDEBUG_EVENT lpDebugEvent,
/* Setup the thread data */ /* Setup the thread data */
SaveThreadHandle(lpDebugEvent->dwProcessId, SaveThreadHandle(lpDebugEvent->dwProcessId,
lpDebugEvent->dwThreadId, lpDebugEvent->dwThreadId,
lpDebugEvent->u.CreateThread.hThread); lpDebugEvent->u.CreateProcessInfo.hThread);
break; break;
/* Process was exited */ /* Process was exited */
case EXIT_PROCESS_DEBUG_EVENT: case EXIT_PROCESS_DEBUG_EVENT:
/* Mark the thread data as such */ /* Mark the thread data as such and fall through */
MarkProcessHandle(lpDebugEvent->dwProcessId); MarkProcessHandle(lpDebugEvent->dwProcessId);
break;
/* Thread was exited */ /* Thread was exited */
case EXIT_THREAD_DEBUG_EVENT: case EXIT_THREAD_DEBUG_EVENT:
@ -727,10 +693,18 @@ WaitForDebugEvent(IN LPDEBUG_EVENT lpDebugEvent,
/* Mark the thread data */ /* Mark the thread data */
MarkThreadHandle(lpDebugEvent->dwThreadId); MarkThreadHandle(lpDebugEvent->dwThreadId);
break; break;
/* Nothing to do for anything else */ /* Nothing to do */
default: case EXCEPTION_DEBUG_EVENT:
case LOAD_DLL_DEBUG_EVENT:
case UNLOAD_DLL_DEBUG_EVENT:
case OUTPUT_DEBUG_STRING_EVENT:
case RIP_EVENT:
break; break;
/* Fail anything else */
default:
return FALSE;
} }
/* Return success */ /* Return success */
@ -957,29 +931,24 @@ OutputDebugStringA(IN LPCSTR _OutputString)
*/ */
VOID VOID
WINAPI WINAPI
OutputDebugStringW(IN LPCWSTR _OutputString) OutputDebugStringW(IN LPCWSTR OutputString)
{ {
UNICODE_STRING wstrOut; UNICODE_STRING UnicodeString;
ANSI_STRING strOut; ANSI_STRING AnsiString;
NTSTATUS nErrCode; NTSTATUS Status;
/* convert the string in ANSI */ /* convert the string in ANSI */
RtlInitUnicodeString(&wstrOut, _OutputString); RtlInitUnicodeString(&UnicodeString, OutputString);
nErrCode = RtlUnicodeStringToAnsiString(&strOut, &wstrOut, TRUE); Status = RtlUnicodeStringToAnsiString(&AnsiString, &UnicodeString, TRUE);
if(!NT_SUCCESS(nErrCode)) /* OutputDebugStringW always prints something, even if conversion fails */
{ if (!NT_SUCCESS(Status)) AnsiString.Buffer = "";
/* Microsoft's kernel32.dll always prints something, even in case the conversion fails */
OutputDebugStringA("");
}
else
{
/* output the converted string */
OutputDebugStringA(strOut.Buffer);
/* free the converted string */ /* Output the converted string */
RtlFreeAnsiString(&strOut); OutputDebugStringA(AnsiString.Buffer);
}
/* free the converted string */
if (NT_SUCCESS(Status)) RtlFreeAnsiString(&AnsiString);
} }
/* EOF */ /* EOF */