[ros-diffs] [ion] 25408: - 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.
ion at svn.reactos.org
ion at svn.reactos.org
Wed Jan 10 04:36:00 CET 2007
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=25408&r1=25407&r2=25408&view=diff
==============================================================================
--- 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/ob.h?rev=25408&r1=25407&r2=25408&view=diff
==============================================================================
--- 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=25408&r1=25407&r2=25408&view=diff
==============================================================================
--- 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=25408&r1=25407&r2=25408&view=diff
==============================================================================
--- 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
More information about the Ros-diffs
mailing list