Author: ion Date: Wed Jan 10 06:35:59 2007 New Revision: 25408
URL: http://svn.reactos.org/svn/reactos?rev=25408&view=rev Log: - Implement ObReferenceProcessHandleTable and ObDereferenceProcessHandleTable and use them where appropriate to avoid race issues if the process is being killed meanwhile. - Implement ObpReferenceProcessObjectByHandle and simplfy ObDuplicateObject. - Disable hard errors while closing handles, and protect against races. Also print our error message since it seems handles aren't being closed now (message displays leak count). - Honour DUPLICATE_CLOSE_SOURCE during failure paths in ObDuplicateObject, and catch race conditions. - Add some more sanity checks and speed up some internal referencing.
Modified: trunk/reactos/ntoskrnl/KrnlFun.c trunk/reactos/ntoskrnl/include/internal/ob.h trunk/reactos/ntoskrnl/ob/obhandle.c trunk/reactos/ntoskrnl/ob/obref.c
Modified: trunk/reactos/ntoskrnl/KrnlFun.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/KrnlFun.c?rev=2540... ============================================================================== --- trunk/reactos/ntoskrnl/KrnlFun.c (original) +++ trunk/reactos/ntoskrnl/KrnlFun.c Wed Jan 10 06:35:59 2007 @@ -7,11 +7,6 @@ // Do NOT complain about it. // Do NOT ask when it will be fixed. // Failure to respect this will *ACHIEVE NOTHING*. -// -// -// Ob: -// - Add Directory Lock. -// - Add Object Table Referencing. // // Ex: // - Fixup existing code that talks to Ke.
Modified: trunk/reactos/ntoskrnl/include/internal/ob.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/o... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/ob.h (original) +++ trunk/reactos/ntoskrnl/include/internal/ob.h Wed Jan 10 06:35:59 2007 @@ -171,6 +171,18 @@ IN PEPROCESS Process );
+PHANDLE_TABLE +NTAPI +ObReferenceProcessHandleTable( + IN PEPROCESS Process +); + +VOID +NTAPI +ObDereferenceProcessHandleTable( + IN PEPROCESS Process +); + VOID NTAPI ObKillProcess(
Modified: trunk/reactos/ntoskrnl/ob/obhandle.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=... ============================================================================== --- trunk/reactos/ntoskrnl/ob/obhandle.c (original) +++ trunk/reactos/ntoskrnl/ob/obhandle.c Wed Jan 10 06:35:59 2007 @@ -22,6 +22,165 @@ #define TAG_OB_HANDLE TAG('O', 'b', 'H', 'd')
/* PRIVATE FUNCTIONS *********************************************************/ + +PHANDLE_TABLE +NTAPI +ObReferenceProcessHandleTable(IN PEPROCESS Process) +{ + PHANDLE_TABLE HandleTable = NULL; + + /* Lock the process */ + if (ExAcquireRundownProtection(&Process->RundownProtect)) + { + /* Get the handle table */ + HandleTable = Process->ObjectTable; + if (!HandleTable) + { + /* No table, release the lock */ + ExReleaseRundownProtection(&Process->RundownProtect); + } + } + + /* Return the handle table */ + return HandleTable; +} + +VOID +NTAPI +ObDereferenceProcessHandleTable(IN PEPROCESS Process) +{ + /* Release the process lock */ + ExReleaseRundownProtection(&Process->RundownProtect); +} + +NTSTATUS +NTAPI +ObpReferenceProcessObjectByHandle(IN HANDLE Handle, + IN PEPROCESS Process, + IN PHANDLE_TABLE HandleTable, + IN KPROCESSOR_MODE AccessMode, + OUT PVOID *Object, + OUT POBJECT_HANDLE_INFORMATION HandleInformation) +{ + PHANDLE_TABLE_ENTRY HandleEntry; + POBJECT_HEADER ObjectHeader; + ACCESS_MASK GrantedAccess; + ULONG Attributes; + PEPROCESS CurrentProcess; + PETHREAD CurrentThread; + NTSTATUS Status; + PAGED_CODE(); + + /* Assume failure */ + *Object = NULL; + + /* Check if the caller wants the current process */ + if (Handle == NtCurrentProcess()) + { + /* Get the current process */ + CurrentProcess = PsGetCurrentProcess(); + + /* Check if the caller wanted handle information */ + if (HandleInformation) + { + /* Return it */ + HandleInformation->HandleAttributes = 0; + HandleInformation->GrantedAccess = Process->GrantedAccess; + } + + /* Reference ourselves */ + ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentProcess); + InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + + /* Return the pointer */ + *Object = CurrentProcess; + return STATUS_SUCCESS; + } + + /* Check if the caller wants the current thread */ + if (Handle == NtCurrentThread()) + { + /* Get the current thread */ + CurrentThread = PsGetCurrentThread(); + + /* Check if the caller wanted handle information */ + if (HandleInformation) + { + /* Return it */ + HandleInformation->HandleAttributes = 0; + HandleInformation->GrantedAccess = CurrentThread->GrantedAccess; + } + + /* Reference ourselves */ + ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentThread); + InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + + /* Return the pointer */ + *Object = CurrentThread; + return STATUS_SUCCESS; + } + + /* Check if this is a kernel handle */ + if (ObIsKernelHandle(Handle, AccessMode)) + { + /* Use the kernel handle table and get the actual handle value */ + Handle = ObKernelHandleToHandle(Handle); + HandleTable = ObpKernelHandleTable; + } + else + { + /* Otherwise use this process's handle table */ + HandleTable = PsGetCurrentProcess()->ObjectTable; + } + + /* Enter a critical region while we touch the handle table */ + ASSERT(HandleTable != NULL); + KeEnterCriticalRegion(); + + /* Get the handle entry */ + HandleEntry = ExMapHandleToPointer(HandleTable, Handle); + if (HandleEntry) + { + /* Get the object header and validate the type*/ + ObjectHeader = EX_HTE_TO_HDR(HandleEntry); + + /* Get the granted access and validate it */ + GrantedAccess = HandleEntry->GrantedAccess; + + /* Mask out the internal attributes */ + Attributes = HandleEntry->ObAttributes & + (EX_HANDLE_ENTRY_PROTECTFROMCLOSE | + EX_HANDLE_ENTRY_INHERITABLE | + EX_HANDLE_ENTRY_AUDITONCLOSE); + + /* Fill out the information */ + HandleInformation->HandleAttributes = Attributes; + HandleInformation->GrantedAccess = GrantedAccess; + + /* Return the pointer */ + *Object = &ObjectHeader->Body; + + /* Add a reference */ + InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + + /* Unlock the handle */ + ExUnlockHandleTableEntry(HandleTable, HandleEntry); + KeLeaveCriticalRegion(); + + /* Return success */ + ASSERT(*Object != NULL); + return STATUS_SUCCESS; + } + else + { + /* Invalid handle */ + Status = STATUS_INVALID_HANDLE; + } + + /* Return failure status */ + KeLeaveCriticalRegion(); + return Status; +}
BOOLEAN NTAPI @@ -1741,11 +1900,15 @@ /* Check if we have a parent */ if (Parent) { + /* Get the parent's table */ + HandleTable = ObReferenceProcessHandleTable(Parent); + if (!HandleTable) return STATUS_PROCESS_IS_TERMINATING; + /* Duplicate the parent's */ HandleTable = ExDupHandleTable(Process, ObpDuplicateHandleCallback, NULL, - Parent->ObjectTable); + HandleTable); } else { @@ -1753,11 +1916,14 @@ HandleTable = ExCreateHandleTable(Process); }
- /* Now write it and make sure we got one */ + /* Now write it */ Process->ObjectTable = HandleTable; + + /* Dereference the parent's handle table if we have one */ + if (Parent) ObDereferenceProcessHandleTable(Parent); + + /* Fail or succeed depending on whether we got a handle table or not */ if (!HandleTable) return STATUS_INSUFFICIENT_RESOURCES; - - /* If we got here then the table was created OK */ return STATUS_SUCCESS; }
@@ -1778,9 +1944,20 @@ NTAPI ObKillProcess(IN PEPROCESS Process) { - PHANDLE_TABLE HandleTable = Process->ObjectTable; + PHANDLE_TABLE HandleTable; OBP_CLOSE_HANDLE_CONTEXT Context; + BOOLEAN HardErrors; PAGED_CODE(); + + /* Wait for process rundown */ + ExWaitForRundownProtectionRelease(&Process->RundownProtect); + + /* Get the object table */ + HandleTable = Process->ObjectTable; + if (!HandleTable) return; + + /* Disable hard errors while we close handles */ + HardErrors = IoSetThreadHardErrorMode(FALSE);
/* Enter a critical region */ KeEnterCriticalRegion(); @@ -1793,13 +1970,20 @@ ExSweepHandleTable(HandleTable, ObpCloseHandleCallback, &Context); - - /* Destroy the table and leave the critical region */ + if (HandleTable->HandleCount != 0) + { + DPRINT1("FIXME: %d handles remain!\n", HandleTable->HandleCount); + } + + /* Leave the critical region */ + KeLeaveCriticalRegion(); + + /* Re-enable hard errors */ + IoSetThreadHardErrorMode(HardErrors); + + /* Destroy the object table */ + Process->ObjectTable = NULL; ExDestroyHandleTable(HandleTable); - KeLeaveCriticalRegion(); - - /* Clear the object table */ - Process->ObjectTable = NULL; }
NTSTATUS @@ -1820,12 +2004,12 @@ POBJECT_TYPE ObjectType; HANDLE NewHandle; KAPC_STATE ApcState; - NTSTATUS Status = STATUS_SUCCESS; + NTSTATUS Status; ACCESS_MASK TargetAccess, SourceAccess; ACCESS_STATE AccessState; PACCESS_STATE PassedAccessState = NULL; AUX_DATA AuxData; - PHANDLE_TABLE HandleTable = NULL; + PHANDLE_TABLE HandleTable; OBJECT_HANDLE_INFORMATION HandleInformation; PAGED_CODE(); OBTRACE(OB_HANDLE_DEBUG, @@ -1835,32 +2019,77 @@ SourceProcess, TargetProcess);
- /* Check if we're not in the source process */ - if (SourceProcess != PsGetCurrentProcess()) - { - /* Attach to it */ - KeStackAttachProcess(&SourceProcess->Pcb, &ApcState); - AttachedToProcess = TRUE; - } - - /* Now reference the source handle */ - Status = ObReferenceObjectByHandle(SourceHandle, - 0, - NULL, - PreviousMode, - (PVOID*)&SourceObject, - &HandleInformation); - - /* Check if we were attached */ - if (AttachedToProcess) - { - /* We can safely detach now */ - KeUnstackDetachProcess(&ApcState); - AttachedToProcess = FALSE; - } - - /* Fail if we couldn't reference it */ - if (!NT_SUCCESS(Status)) return Status; + /* Check if we're not duplicating the same access */ + if (!(Options & DUPLICATE_SAME_ACCESS)) + { + /* Validate the desired access */ + Status = STATUS_SUCCESS; //ObpValidateDesiredAccess(DesiredAccess); + if (!NT_SUCCESS(Status)) return Status; + } + + /* Reference the object table */ + HandleTable = ObReferenceProcessHandleTable(SourceProcess); + if (!HandleTable) return STATUS_PROCESS_IS_TERMINATING; + + /* Reference the process object */ + Status = ObpReferenceProcessObjectByHandle(SourceHandle, + 0, + HandleTable, + PreviousMode, + &SourceObject, + &HandleInformation); + if (!NT_SUCCESS(Status)) + { + /* Fail */ + ObDereferenceProcessHandleTable(SourceProcess); + return Status; + } + + /* Check if there's no target process */ + if (!TargetProcess) + { + /* Check if the caller wanted actual duplication */ + if (!(Options & DUPLICATE_CLOSE_SOURCE)) + { + /* Invalid request */ + Status = STATUS_INVALID_PARAMETER; + } + else + { + /* Otherwise, do the attach */ + KeStackAttachProcess(&SourceProcess->Pcb, &ApcState); + + /* Close the handle and detach */ + NtClose(SourceHandle); + KeUnstackDetachProcess(&ApcState); + } + + /* Return */ + ObDereferenceProcessHandleTable(SourceProcess); + ObDereferenceObject(SourceObject); + return Status; + } + + /* Get the target handle table */ + HandleTable = ObReferenceProcessHandleTable(TargetProcess); + if (!HandleTable) + { + /* Check if the caller wanted us to close the handle */ + if (Options & DUPLICATE_CLOSE_SOURCE) + { + /* Do the attach */ + KeStackAttachProcess(&SourceProcess->Pcb, &ApcState); + + /* Close the handle and detach */ + NtClose(SourceHandle); + KeUnstackDetachProcess(&ApcState); + } + + /* Return */ + ObDereferenceProcessHandleTable(SourceProcess); + ObDereferenceObject(SourceObject); + return STATUS_PROCESS_IS_TERMINATING; + }
/* Get the source access */ SourceAccess = HandleInformation.GrantedAccess; @@ -1898,7 +2127,8 @@ if (DesiredAccess & GENERIC_ACCESS) { /* Map it */ - RtlMapGenericMask(&DesiredAccess, &ObjectType->TypeInfo.GenericMapping); + RtlMapGenericMask(&DesiredAccess, + &ObjectType->TypeInfo.GenericMapping); }
/* Set the target access */ @@ -1940,9 +2170,6 @@ HandleAttributes, PsGetCurrentProcess(), ObDuplicateHandle); - - /* Set the handle table, now that we know this handle was added */ - HandleTable = PsGetCurrentProcess()->ObjectTable; }
/* Check if we were attached */ @@ -1989,6 +2216,10 @@
/* Return the handle */ if (TargetHandle) *TargetHandle = NewHandle; + + /* Dereference handle tables */ + ObDereferenceProcessHandleTable(SourceProcess); + ObDereferenceProcessHandleTable(TargetProcess);
/* Return status */ OBTRACE(OB_HANDLE_DEBUG,
Modified: trunk/reactos/ntoskrnl/ob/obref.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obref.c?rev=254... ============================================================================== --- trunk/reactos/ntoskrnl/ob/obref.c (original) +++ trunk/reactos/ntoskrnl/ob/obref.c Wed Jan 10 06:35:59 2007 @@ -478,8 +478,8 @@ NTSTATUS Status; PAGED_CODE();
- /* Fail immediately if the handle is NULL */ - if (!Handle) return STATUS_INVALID_HANDLE; + /* Assume failure */ + *Object = NULL;
/* Check if the caller wants the current process */ if ((Handle == NtCurrentProcess()) && @@ -488,9 +488,6 @@ /* Get the current process */ CurrentProcess = PsGetCurrentProcess();
- /* Reference ourselves */ - ObReferenceObject(CurrentProcess); - /* Check if the caller wanted handle information */ if (HandleInformation) { @@ -499,6 +496,10 @@ HandleInformation->GrantedAccess = PROCESS_ALL_ACCESS; }
+ /* Reference ourselves */ + ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentProcess); + InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + /* Return the pointer */ *Object = CurrentProcess; return STATUS_SUCCESS; @@ -516,9 +517,6 @@ /* Get the current thread */ CurrentThread = PsGetCurrentThread();
- /* Reference ourselves */ - ObReferenceObject(CurrentThread); - /* Check if the caller wanted handle information */ if (HandleInformation) { @@ -527,6 +525,10 @@ HandleInformation->GrantedAccess = THREAD_ALL_ACCESS; }
+ /* Reference ourselves */ + ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentThread); + InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + /* Return the pointer */ *Object = CurrentThread; return STATUS_SUCCESS; @@ -551,6 +553,7 @@ }
/* Enter a critical region while we touch the handle table */ + ASSERT(HandleTable != NULL); KeEnterCriticalRegion();
/* Get the handle entry */ @@ -588,9 +591,10 @@
/* Unlock the handle */ ExUnlockHandleTableEntry(HandleTable, HandleEntry); + KeLeaveCriticalRegion();
/* Return success */ - KeLeaveCriticalRegion(); + ASSERT(*Object != NULL); return STATUS_SUCCESS; } else