[ion] 25799: Object Manager fixes based on bug reports from Aleksey Bragin: - ObpReferenceProcessObjectByHandle: - Remove PAGED_CODE - Use InterlockedIncrement instead of InterlockedExchangeAdd. - ObpInsertHandleCount: Fix calculation of the handle database size, fixing potential pool corruption/overwrite situations. - ObpChargeQuotaForObject: Write proper code for charging the quota. - ObpDecrementHandleCount: - Get ObjectType from caller instead of doing the extra operation. - If there's no ha
Author: ion Date: Wed Feb 14 20:51:48 2007 New Revision: 25799
URL: http://svn.reactos.org/svn/reactos?rev=25799&view=rev Log: Object Manager fixes based on bug reports from Aleksey Bragin: - ObpReferenceProcessObjectByHandle: - Remove PAGED_CODE - Use InterlockedIncrement instead of InterlockedExchangeAdd. - ObpInsertHandleCount: Fix calculation of the handle database size, fixing potential pool corruption/overwrite situations. - ObpChargeQuotaForObject: Write proper code for charging the quota. - ObpDecrementHandleCount: - Get ObjectType from caller instead of doing the extra operation. - If there's no handle database, then don't setup a handle entry. - ObpIncrementHandleCount is PAGED_CODE. - ObpCloseHandle: Do proper logic for determining if we should raise an exception, fail, or bugcheck the system when an invalid handle is being closed. - ObpSetHandleAttributes: - Not PAGED_CODE. - Allow operations on kernel objects. - Use the Access Protect Close Bit in the GrantedAccess instead of an OBJ_PROTECT flag in the ObAttributes. - ObpCloseHandleCallback: Sweep/Enumerate Routines are BOOLEAN, not VOID. - ObDuplicateObject: - Clear the audit mask if auditing isn't set. - Always duplicate OBJ_AUDIT_OBJECT_CLOSE if it's set. - Clear the handle entry before writing it. - Always propagate ACCESS_SYSTEM_SECURITY as a desired access. - ObFindHandleForObject: Use ObReferenceProcessHandleTable instead of directly accessing the pointer. - ObInsertObject: Dereference symbolic links when they collide during an insert, since a reference was already added. - NtDuplicateObject: Clear out the TargetHandle to assume failure. - ObpCaptureObjectName: Use RtlCopyMemory which is faster. - ObpAllocateObject: Fix check for quota usage. - ObCreateObjectType: - Make sure that the object type name is wchar-aligned. - Add support for LPC Waitable Ports. - Initialize Object Locks.
Modified: trunk/reactos/ntoskrnl/include/internal/ex.h trunk/reactos/ntoskrnl/ob/obhandle.c trunk/reactos/ntoskrnl/ob/oblife.c
Modified: trunk/reactos/ntoskrnl/include/internal/ex.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/e... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/ex.h (original) +++ trunk/reactos/ntoskrnl/include/internal/ex.h Wed Feb 14 20:51:48 2007 @@ -336,7 +336,7 @@
/* HANDLE TABLE FUNCTIONS ***************************************************/
-typedef VOID +typedef BOOLEAN (NTAPI *PEX_SWEEP_HANDLE_CALLBACK)( PHANDLE_TABLE_ENTRY HandleTableEntry, HANDLE Handle,
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 Feb 14 20:51:48 2007 @@ -15,9 +15,10 @@
#include <ntoskrnl.h> #define NDEBUG -#include <internal/debug.h> +#include <debug.h>
PHANDLE_TABLE ObpKernelHandleTable = NULL; +ULONG ObpAccessProtectCloseBit = MAXIMUM_ALLOWED;
#define TAG_OB_HANDLE TAG('O', 'b', 'H', 'd')
@@ -69,7 +70,6 @@ ULONG Attributes; PETHREAD Thread = PsGetCurrentThread(); NTSTATUS Status; - PAGED_CODE();
/* Assume failure */ *Object = NULL; @@ -86,7 +86,7 @@
/* Reference ourselves */ ObjectHeader = OBJECT_TO_OBJECT_HEADER(Process); - InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1); + InterlockedIncrement(&ObjectHeader->PointerCount);
/* Return the pointer */ *Object = Process; @@ -249,7 +249,7 @@ /* Now we'll have two entries, and an entire DB */ i = 2; Size = sizeof(OBJECT_HANDLE_COUNT_DATABASE) + - sizeof(OBJECT_HANDLE_COUNT_ENTRY); + ((i - 1) * sizeof(OBJECT_HANDLE_COUNT_ENTRY)); } else { @@ -261,7 +261,7 @@
/* Add 4 more entries */ i += 4; - Size = OldSize += (4 * sizeof(OBJECT_HANDLE_COUNT_ENTRY)); + Size = OldSize += ((i - 1) * sizeof(OBJECT_HANDLE_COUNT_ENTRY)); }
/* Allocate the DB */ @@ -395,7 +395,6 @@ { POBJECT_HEADER_QUOTA_INFO ObjectQuota; ULONG PagedPoolCharge, NonPagedPoolCharge; - PEPROCESS Process;
/* Get quota information */ ObjectQuota = OBJECT_HEADER_TO_QUOTA_INFO(ObjectHeader); @@ -404,9 +403,6 @@ /* Check if this is a new object */ if (ObjectHeader->Flags & OB_FLAG_CREATE_INFO) { - /* Set the flag */ - *NewObject = TRUE; - /* Remove the flag */ ObjectHeader->Flags &= ~ OB_FLAG_CREATE_INFO; if (ObjectQuota) @@ -422,14 +418,19 @@ NonPagedPoolCharge = ObjectType->TypeInfo.DefaultNonPagedPoolCharge; }
- /* - * Charge the quota - * FIXME: This is a *COMPLETE* guess and probably defintely not the way to do this. - */ - Process = PsGetCurrentProcess(); - Process->QuotaBlock->QuotaEntry[PagedPool].Usage += PagedPoolCharge; - Process->QuotaBlock->QuotaEntry[NonPagedPool].Usage += NonPagedPoolCharge; - ObjectHeader->QuotaBlockCharged = Process->QuotaBlock; + /* Charge the quota */ + ObjectHeader->QuotaBlockCharged = (PVOID)1; +#if 0 + PsChargeSharedPoolQuota(PsGetCurrentProcess(), + PagedPoolCharge, + NonPagedPoolCharge); +#endif + + /* Check if we don't have a quota block */ + if (!ObjectHeader->QuotaBlockCharged) return STATUS_QUOTA_EXCEEDED; + + /* Now set the flag */ + *NewObject = TRUE; }
/* Return success */ @@ -459,10 +460,10 @@ NTAPI ObpDecrementHandleCount(IN PVOID ObjectBody, IN PEPROCESS Process, - IN ACCESS_MASK GrantedAccess) + IN ACCESS_MASK GrantedAccess, + IN POBJECT_TYPE ObjectType) { POBJECT_HEADER ObjectHeader; - POBJECT_TYPE ObjectType; LONG SystemHandleCount, ProcessHandleCount; LONG NewCount; KIRQL CalloutIrql; @@ -474,7 +475,6 @@
/* Get the object type and header */ ObjectHeader = OBJECT_TO_OBJECT_HEADER(ObjectBody); - ObjectType = ObjectHeader->Type; OBTRACE(OB_HANDLE_DEBUG, "%s - Decrementing count for: %p. HC LC %lx %lx\n", __FUNCTION__, @@ -540,6 +540,11 @@ i--; } } + else + { + /* No database, so no entry */ + HandleEntry = NULL; + } }
/* Check if this is the last handle */ @@ -686,7 +691,10 @@ ExDestroyHandle(HandleTable, Handle, HandleEntry);
/* Now decrement the handle count */ - ObpDecrementHandleCount(Body, PsGetCurrentProcess(), GrantedAccess); + ObpDecrementHandleCount(Body, + PsGetCurrentProcess(), + GrantedAccess, + ObjectType);
/* Dereference the object as well */ ObDereferenceObject(Body); @@ -749,6 +757,7 @@ KIRQL CalloutIrql; KPROCESSOR_MODE ProbeMode; ULONG Total; + PAGED_CODE();
/* Get the object header and type */ ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object); @@ -1359,7 +1368,8 @@ /* Decrement the handle count and detach */ ObpDecrementHandleCount(&ObjectHeader->Body, PsGetCurrentProcess(), - GrantedAccess); + GrantedAccess, + ObjectType);
/* Detach and fail */ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); @@ -1596,7 +1606,8 @@ /* Decrement the handle count and detach */ ObpDecrementHandleCount(&ObjectHeader->Body, PsGetCurrentProcess(), - GrantedAccess); + GrantedAccess, + ObjectType);
/* Handle extra references */ if (AdditionalReferences) @@ -1644,6 +1655,7 @@ KAPC_STATE ApcState; PHANDLE_TABLE_ENTRY HandleTableEntry; NTSTATUS Status; + PEPROCESS Process = PsGetCurrentProcess(); PAGED_CODE(); OBTRACE(OB_HANDLE_DEBUG, "%s - Closing handle: %lx\n", __FUNCTION__, Handle); @@ -1656,7 +1668,7 @@ Handle = ObKernelHandleToHandle(Handle);
/* Check if we're not in the system process */ - if (PsGetCurrentProcess() != PsInitialSystemProcess) + if (Process != PsInitialSystemProcess) { /* Attach to the system process */ KeStackAttachProcess(&PsInitialSystemProcess->Pcb, &ApcState); @@ -1666,7 +1678,7 @@ else { /* Use the process's handle table */ - HandleTable = PsGetCurrentProcess()->ObjectTable; + HandleTable = Process->ObjectTable; }
/* Enter a critical region to protect handle access */ @@ -1698,18 +1710,43 @@ /* Detach */ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
- /* Check if this was a user-mode caller with a valid debug port */ - if ((AccessMode != KernelMode) && - (PsGetCurrentProcess()->DebugPort)) - { - /* Raise an exception */ - Status = KeRaiseUserException(STATUS_INVALID_HANDLE); - } - else - { - /* Just return the status */ - Status = STATUS_INVALID_HANDLE; - } + /* Check if we have a valid handle that's not the process or thread */ + if ((Handle) && + (Handle != NtCurrentProcess()) && + (Handle != NtCurrentThread())) + { + /* Check if we came from user mode */ + if (AccessMode != KernelMode) + { + /* Check if we have no debug port */ + if (Process->DebugPort) + { + /* Make sure we're not attached */ + if (!KeIsAttachedProcess()) + { + /* Raise an exception */ + return KeRaiseUserException(STATUS_INVALID_HANDLE); + } + } + } + else + { + /* This is kernel mode. Check if we're exiting */ + if (!(PsIsThreadTerminating(PsGetCurrentThread())) && + (Process->Peb)) + { + /* Check if the debugger is enabled */ + if (KdDebuggerEnabled) + { + /* Bugcheck */ + KeBugCheckEx(0, (ULONG_PTR)Handle, 1, 0, 0); + } + } + } + } + + /* Set invalid status */ + Status = STATUS_INVALID_HANDLE; }
/* Return status */ @@ -1742,15 +1779,6 @@ { POBP_SET_HANDLE_ATTRIBUTES_CONTEXT SetHandleInfo = (PVOID)Context; POBJECT_HEADER ObjectHeader = ObpGetHandleObject(HandleTableEntry); - PAGED_CODE(); - - /* Don't allow operations on kernel objects */ - if ((ObjectHeader->Flags & OB_FLAG_KERNEL_MODE) && - (SetHandleInfo->PreviousMode != KernelMode)) - { - /* Fail */ - return FALSE; - }
/* Check if making the handle inheritable */ if (SetHandleInfo->Information.Inherit) @@ -1775,12 +1803,12 @@ if (SetHandleInfo->Information.ProtectFromClose) { /* Set the flag */ - HandleTableEntry->ObAttributes |= OBJ_PROTECT_CLOSE; + HandleTableEntry->GrantedAccess |= ObpAccessProtectCloseBit; } else { /* Otherwise, remove it */ - HandleTableEntry->ObAttributes &= ~OBJ_PROTECT_CLOSE; + HandleTableEntry->GrantedAccess &= ~ObpAccessProtectCloseBit; }
/* Return success */ @@ -1809,7 +1837,7 @@ * @remarks None. * *--*/ -VOID +BOOLEAN NTAPI ObpCloseHandleCallback(IN PHANDLE_TABLE_ENTRY HandleTableEntry, IN HANDLE Handle, @@ -1823,6 +1851,7 @@ Handle, CloseContext->AccessMode, TRUE); + return TRUE; }
/*++ @@ -2078,6 +2107,15 @@ ObDereferenceProcessHandleTable(SourceProcess); return Status; } + else + { + /* Check if we have to don't have to audit object close */ + if (!(HandleInformation.HandleAttributes & OBJ_AUDIT_OBJECT_CLOSE)) + { + /* Then there is no audit mask */ + AuditMask = 0; + } + }
/* Check if there's no target process */ if (!TargetProcess) @@ -2142,6 +2180,12 @@ /* Duplicate them */ HandleAttributes = HandleInformation.HandleAttributes; } + else + { + /* Don't allow caller to bypass auditing */ + HandleAttributes |= HandleInformation.HandleAttributes & + OBJ_AUDIT_OBJECT_CLOSE; + }
/* Check if we're duplicating the access */ if (Options & DUPLICATE_SAME_ACCESS) DesiredAccess = SourceAccess; @@ -2151,6 +2195,7 @@ ObjectType = ObjectHeader->Type;
/* Fill out the entry */ + RtlZeroMemory(&NewHandleEntry, sizeof(HANDLE_TABLE_ENTRY)); NewHandleEntry.Object = ObjectHeader; NewHandleEntry.ObAttributes |= HandleAttributes & OBJ_HANDLE_ATTRIBUTES;
@@ -2162,8 +2207,9 @@ &ObjectType->TypeInfo.GenericMapping); }
- /* Set the target access */ - TargetAccess = DesiredAccess; + /* Set the target access, always propagate ACCESS_SYSTEM_SECURITY */ + TargetAccess = DesiredAccess & (ObjectType->TypeInfo.ValidAccessMask | + ACCESS_SYSTEM_SECURITY); NewHandleEntry.GrantedAccess = TargetAccess;
/* Check if we're asking for new access */ @@ -2242,7 +2288,8 @@ /* Undo the increment */ ObpDecrementHandleCount(SourceObject, TargetProcess, - TargetAccess); + TargetAccess, + ObjectType);
/* Deference the object and set failure status */ ObDereferenceObject(SourceObject); @@ -2623,9 +2670,11 @@ { OBP_FIND_HANDLE_DATA FindData; BOOLEAN Result = FALSE; + PVOID ObjectTable;
/* Make sure we have an object table */ - if (Process->ObjectTable) + ObjectTable = ObReferenceProcessHandleTable(Process); + if (ObjectTable) { /* Check if we have an object */ if (Object) @@ -2652,6 +2701,9 @@ /* Set success */ Result = TRUE; } + + /* Let go of the table */ + ObDereferenceProcessHandleTable(Process); }
/* Return the result */ @@ -2859,6 +2911,14 @@ } else { + /* Check if this was a symbolic link */ + if (OBJECT_TO_OBJECT_HEADER(InsertObject)->Type == + ObSymbolicLinkType) + { + /* Dereference it */ + ObDereferenceObject(InsertObject); + } + /* Caller wanted to create a new object, fail */ Status = STATUS_OBJECT_NAME_COLLISION; } @@ -3094,7 +3154,6 @@ HANDLE hTarget; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS; - PAGED_CODE(); OBTRACE(OB_HANDLE_DEBUG, "%s - Duplicating handle: %lx for %lx into %lx.\n", __FUNCTION__, @@ -3108,8 +3167,9 @@ /* Enter SEH */ _SEH_TRY { - /* Probe the handle */ + /* Probe the handle and assume failure */ ProbeForWriteHandle(TargetHandle); + *TargetHandle = NULL; } _SEH_HANDLE {
Modified: trunk/reactos/ntoskrnl/ob/oblife.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/oblife.c?rev=25... ============================================================================== --- trunk/reactos/ntoskrnl/ob/oblife.c (original) +++ trunk/reactos/ntoskrnl/ob/oblife.c Wed Feb 14 20:51:48 2007 @@ -398,7 +398,7 @@ else { /* Copy the name */ - RtlMoveMemory(StringBuffer, LocalName.Buffer, StringLength); + RtlCopyMemory(StringBuffer, LocalName.Buffer, StringLength); StringBuffer[StringLength / sizeof(WCHAR)] = UNICODE_NULL; } } @@ -603,11 +603,12 @@ else { /* Check if we have quota */ - if ((ObjectCreateInfo->PagedPoolCharge != - ObjectType->TypeInfo.DefaultPagedPoolCharge) || - (ObjectCreateInfo->NonPagedPoolCharge != - ObjectType->TypeInfo.DefaultNonPagedPoolCharge) || - (ObjectCreateInfo->SecurityDescriptorCharge > 2048) || + if ((((ObjectCreateInfo->PagedPoolCharge != + ObjectType->TypeInfo.DefaultPagedPoolCharge) || + (ObjectCreateInfo->NonPagedPoolCharge != + ObjectType->TypeInfo.DefaultNonPagedPoolCharge) || + (ObjectCreateInfo->SecurityDescriptorCharge > 2048)) && + (PsGetCurrentProcess() != PsInitialSystemProcess)) || (ObjectCreateInfo->Attributes & OBJ_EXCLUSIVE)) { /* Set quota size */ @@ -1010,6 +1011,7 @@ /* Verify parameters */ if (!(TypeName) || !(TypeName->Length) || + (TypeName->Length % sizeof(WCHAR)) || !(ObjectTypeInitializer) || (ObjectTypeInitializer->Length != sizeof(*ObjectTypeInitializer)) || (ObjectTypeInitializer->InvalidAttributes & ~OBJ_VALID_ATTRIBUTES) || @@ -1167,7 +1169,12 @@ LocalObjectType->DefaultObject = (PVOID)FIELD_OFFSET(FILE_OBJECT, Event); } - /* FIXME: When LPC stops sucking, add a hack for Waitable Ports */ + else if ((TypeName->Length == 24) && !(wcscmp(TypeName->Buffer, L"WaitablePort"))) + { + /* Wait on the LPC Port's object directly */ + LocalObjectType->DefaultObject = (PVOID)FIELD_OFFSET(LPCP_PORT_OBJECT, + WaitEvent); + } else { /* No default Object */ @@ -1176,6 +1183,11 @@
/* Initialize Object Type components */ ExInitializeResourceLite(&LocalObjectType->Mutex); + for (i = 0; i < 4; i++) + { + /* Initialize the object locks */ + ExInitializeResourceLite(&LocalObjectType->ObjectLocks[i]); + } InitializeListHead(&LocalObjectType->TypeList);
/* Lock the object type */
-
ion＠svn.reactos.org