Author: ion Date: Tue Jan 9 20:18:22 2007 New Revision: 25400
URL: http://svn.reactos.org/svn/reactos?rev=25400&view=rev Log: - Fix Port and Section Object Type creation by specifying a valid ValidAccessMask when creating the types. - NTLPC "Branch": Ports need to maintain a Type List. - Use proper access mode in parse callbacks. - Properly validate the access mask given to ObpCreate(Unnamed)Handle and only grant valid bits according to ValidAccessMask. - Use InterlockedExchangeAdd for reference count bias instead of looping on a single increment. - Only return the object if the caller did any bias to it. - Detach from the process much later, since exclusive process support and handle table database needs to be in the same context as the owner. - Add audit calls to ObpCreateHandle. - Add stubbed out calls to ObpCleanupDirectoryLookup in ObpCreateHandle.
Modified: trunk/reactos/ntoskrnl/KrnlFun.c trunk/reactos/ntoskrnl/lpc/ntlpc/port.c trunk/reactos/ntoskrnl/lpc/port.c trunk/reactos/ntoskrnl/mm/section.c trunk/reactos/ntoskrnl/ob/obhandle.c trunk/reactos/ntoskrnl/ob/obname.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 Tue Jan 9 20:18:22 2007 @@ -11,8 +11,7 @@ // // Ob: // - Add Directory Lock. -// - Strengthen code with debug checks and assertions. -// - Fix FIXMEs/commented out code. +// - Add Object Table Referencing. // // Ex: // - Fixup existing code that talks to Ke.
Modified: trunk/reactos/ntoskrnl/lpc/ntlpc/port.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/lpc/ntlpc/port.c?r... ============================================================================== --- trunk/reactos/ntoskrnl/lpc/ntlpc/port.c (original) +++ trunk/reactos/ntoskrnl/lpc/ntlpc/port.c Tue Jan 9 20:18:22 2007 @@ -55,6 +55,7 @@ ObjectTypeInitializer.CloseProcedure = LpcpClosePort; ObjectTypeInitializer.DeleteProcedure = LpcpDeletePort; ObjectTypeInitializer.ValidAccessMask = PORT_ALL_ACCESS; + ObjectTypeInitializer.MaintainTypeList = TRUE; ObCreateObjectType(&Name, &ObjectTypeInitializer, NULL,
Modified: trunk/reactos/ntoskrnl/lpc/port.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/lpc/port.c?rev=254... ============================================================================== --- trunk/reactos/ntoskrnl/lpc/port.c (original) +++ trunk/reactos/ntoskrnl/lpc/port.c Tue Jan 9 20:18:22 2007 @@ -56,6 +56,7 @@ ObjectTypeInitializer.UseDefaultObject = TRUE; ObjectTypeInitializer.CloseProcedure = LpcpClosePort; ObjectTypeInitializer.DeleteProcedure = LpcpDeletePort; + ObjectTypeInitializer.ValidAccessMask = PORT_ALL_ACCESS; ObCreateObjectType(&Name, &ObjectTypeInitializer, NULL, &LpcPortObjectType);
LpcpNextMessageId = 0;
Modified: trunk/reactos/ntoskrnl/mm/section.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=2... ============================================================================== --- trunk/reactos/ntoskrnl/mm/section.c (original) +++ trunk/reactos/ntoskrnl/mm/section.c Tue Jan 9 20:18:22 2007 @@ -2277,6 +2277,7 @@ ObjectTypeInitializer.GenericMapping = MmpSectionMapping; ObjectTypeInitializer.DeleteProcedure = MmpDeleteSection; ObjectTypeInitializer.CloseProcedure = MmpCloseSection; + ObjectTypeInitializer.ValidAccessMask = SECTION_ALL_ACCESS; ObCreateObjectType(&Name, &ObjectTypeInitializer, NULL, &MmSectionObjectType);
return(STATUS_SUCCESS);
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 Tue Jan 9 20:18:22 2007 @@ -1031,14 +1031,16 @@ POBJECT_HEADER ObjectHeader; HANDLE Handle; KAPC_STATE ApcState; - BOOLEAN AttachedToProcess = FALSE; + BOOLEAN AttachedToProcess = FALSE, KernelHandle = FALSE; PVOID HandleTable; NTSTATUS Status; - ULONG i; + ACCESS_MASK GrantedAccess; + POBJECT_TYPE ObjectType; PAGED_CODE();
/* Get the object header and type */ ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object); + ObjectType = ObjectHeader->Type; OBTRACE(OB_HANDLE_DEBUG, "%s - Creating handle for: %p. UNNAMED. HC LC %lx %lx\n", __FUNCTION__, @@ -1051,6 +1053,7 @@ { /* Set the handle table */ HandleTable = ObpKernelHandleTable; + KernelHandle = TRUE;
/* Check if we're not in the system process */ if (PsGetCurrentProcess() != PsInitialSystemProcess) @@ -1082,8 +1085,7 @@ return Status; }
- /* Save the object header (assert its validity too) */ - ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED); + /* Save the object header */ NewEntry.Object = ObjectHeader;
/* Mask out the internal attributes */ @@ -1092,20 +1094,20 @@ EX_HANDLE_ENTRY_INHERITABLE | EX_HANDLE_ENTRY_AUDITONCLOSE);
- /* Save the access mask */ - NewEntry.GrantedAccess = DesiredAccess; + /* Remove what's not in the valid access mask */ + GrantedAccess = DesiredAccess & (ObjectType->TypeInfo.ValidAccessMask | + ACCESS_SYSTEM_SECURITY);
/* Handle extra references */ if (AdditionalReferences) { - /* Make a copy in case we fail later below */ - i = AdditionalReferences; - while (i--) - { - /* Increment the count */ - InterlockedIncrement(&ObjectHeader->PointerCount); - } - } + /* Add them to the header */ + InterlockedExchangeAdd(&ObjectHeader->PointerCount, + AdditionalReferences); + } + + /* Save the access mask */ + NewEntry.GrantedAccess = GrantedAccess;
/* * Create the actual handle. We'll need to do this *after* calling @@ -1118,22 +1120,26 @@ NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess); Handle = ExCreateHandle(HandleTable, &NewEntry);
- /* Detach if needed */ - if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); - /* Make sure we got a handle */ if (Handle) { /* Check if this was a kernel handle */ - if (HandleAttributes & OBJ_KERNEL_HANDLE) - { - /* Set the kernel handle bit */ - Handle = ObMarkHandleAsKernelHandle(Handle); - } + if (KernelHandle) Handle = ObMarkHandleAsKernelHandle(Handle);
/* Return handle and object */ *ReturnedHandle = Handle; - if (ReturnedObject) *ReturnedObject = Object; + + /* Return the new object only if caller wanted it biased */ + if ((AdditionalReferences) && (ReturnedObject)) + { + /* Return it */ + *ReturnedObject = Object; + } + + /* Detach if needed */ + if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); + + /* Trace and return */ OBTRACE(OB_HANDLE_DEBUG, "%s - Returning Handle: %lx HC LC %lx %lx\n", __FUNCTION__, @@ -1144,16 +1150,25 @@ }
/* Handle extra references */ - while (AdditionalReferences--) - { - /* Decrement the count */ - InterlockedDecrement(&ObjectHeader->PointerCount); + if (AdditionalReferences == 1) + { + /* Dereference the object once */ + ObDereferenceObject(Object); + } + else if (AdditionalReferences > 1) + { + /* Dereference it many times */ + InterlockedExchangeAdd(&ObjectHeader->PointerCount, + -AdditionalReferences); }
/* Decrement the handle count and detach */ ObpDecrementHandleCount(&ObjectHeader->Body, PsGetCurrentProcess(), - NewEntry.GrantedAccess); + GrantedAccess); + + /* Detach and fail */ + if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); return STATUS_INSUFFICIENT_RESOURCES; }
@@ -1191,7 +1206,7 @@ * * @return <FILLMEIN>. * -* @remarks Cleans up the Lookup Context on success. +* @remarks Cleans up the Lookup Context on return. * *--*/ NTSTATUS @@ -1211,11 +1226,12 @@ POBJECT_HEADER ObjectHeader; HANDLE Handle; KAPC_STATE ApcState; - BOOLEAN AttachedToProcess = FALSE; + BOOLEAN AttachedToProcess = FALSE, KernelHandle = FALSE; POBJECT_TYPE ObjectType; PVOID HandleTable; NTSTATUS Status; - ULONG i; + ACCESS_MASK DesiredAccess, GrantedAccess; + PAUX_DATA AuxData; PAGED_CODE();
/* Get the object header and type */ @@ -1230,13 +1246,19 @@ ObjectHeader->PointerCount);
/* Check if the types match */ - if ((Type) && (ObjectType != Type)) return STATUS_OBJECT_TYPE_MISMATCH; + if ((Type) && (ObjectType != Type)) + { + /* They don't, cleanup */ + //if (Context) ObpCleanupDirectoryLookup(Object, Context); + return STATUS_OBJECT_TYPE_MISMATCH; + }
/* Check if this is a kernel handle */ if ((HandleAttributes & OBJ_KERNEL_HANDLE) && (AccessMode == KernelMode)) { /* Set the handle table */ HandleTable = ObpKernelHandleTable; + KernelHandle = TRUE;
/* Check if we're not in the system process */ if (PsGetCurrentProcess() != PsInitialSystemProcess) @@ -1265,13 +1287,17 @@ * We failed (meaning security failure, according to NT Internals) * detach and return */ + //if (Context) ObpCleanupDirectoryLookup(Object, Context); if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); return Status; }
- /* Save the object header (assert its validity too) */ - ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED); - NewEntry.Object = ObjectHeader; + /* Check if we are doing audits on close */ + if (AccessState->GenerateOnClose) + { + /* Force the attribute on */ + HandleAttributes|= EX_HANDLE_ENTRY_AUDITONCLOSE; + }
/* Mask out the internal attributes */ NewEntry.ObAttributes |= HandleAttributes & @@ -1279,21 +1305,35 @@ EX_HANDLE_ENTRY_INHERITABLE | EX_HANDLE_ENTRY_AUDITONCLOSE);
- /* Save the access mask */ - NewEntry.GrantedAccess = AccessState->RemainingDesiredAccess | - AccessState->PreviouslyGrantedAccess; + /* Get the original desired access */ + DesiredAccess = AccessState->RemainingDesiredAccess | + AccessState->PreviouslyGrantedAccess; + + /* Remove what's not in the valid access mask */ + GrantedAccess = DesiredAccess & (ObjectType->TypeInfo.ValidAccessMask | + ACCESS_SYSTEM_SECURITY); + + /* Update the value in the access state */ + AccessState->PreviouslyGrantedAccess = GrantedAccess; + + /* Get the auxiliary data */ + AuxData = AccessState->AuxData;
/* Handle extra references */ if (AdditionalReferences) { - /* Make a copy in case we fail later below */ - i = AdditionalReferences; - while (i--) - { - /* Increment the count */ - InterlockedIncrement(&ObjectHeader->PointerCount); - } - } + /* Add them to the header */ + InterlockedExchangeAdd(&ObjectHeader->PointerCount, AdditionalReferences); + } + + /* Now we can release the object */ + //if (Context) ObpCleanupDirectoryLookup(Object, Context); + + /* Save the object header */ + NewEntry.Object = ObjectHeader; + + /* Save the access mask */ + NewEntry.GrantedAccess = GrantedAccess;
/* * Create the actual handle. We'll need to do this *after* calling @@ -1306,22 +1346,59 @@ NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess); Handle = ExCreateHandle(HandleTable, &NewEntry);
- /* Detach if needed */ - if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); - /* Make sure we got a handle */ if (Handle) { + /* Check if we have auxiliary data */ + if (AuxData) + { + /* FIXME: Change handle security */ + } + /* Check if this was a kernel handle */ - if (HandleAttributes & OBJ_KERNEL_HANDLE) - { - /* Set the kernel handle bit */ - Handle = ObMarkHandleAsKernelHandle(Handle); - } - - /* Return handle and object */ + if (KernelHandle) Handle = ObMarkHandleAsKernelHandle(Handle); + + /* Return it */ *ReturnedHandle = Handle; - if (ReturnedObject) *ReturnedObject = Object; + + /* Check if we need to generate on audit */ + if (AccessState->GenerateAudit) + { + /* Audit the handle creation */ + //SeAuditHandleCreation(AccessState, Handle); + } + + /* Check if this was a create */ + if (OpenReason == ObCreateHandle) + { + /* Check if we need to audit the privileges */ + if ((AuxData->PrivilegeSet) && + (AuxData->PrivilegeSet->PrivilegeCount)) + { + /* Do the audit */ +#if 0 + SePrivilegeObjectAuditAlarm(Handle, + &AccessState-> + SubjectSecurityContext, + GrantedAccess, + AuxData->PrivilegeSet, + TRUE, + ExGetPreviousMode()); +#endif + } + } + + /* Return the new object only if caller wanted it biased */ + if ((AdditionalReferences) && (ReturnedObject)) + { + /* Return it */ + *ReturnedObject = Object; + } + + /* Detach if needed */ + if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); + + /* Trace and return */ OBTRACE(OB_HANDLE_DEBUG, "%s - Returning Handle: %lx HC LC %lx %lx\n", __FUNCTION__, @@ -1331,17 +1408,26 @@ return STATUS_SUCCESS; }
- /* Handle extra references */ - while (AdditionalReferences--) - { - /* Increment the count */ - InterlockedDecrement(&ObjectHeader->PointerCount); - } - /* Decrement the handle count and detach */ ObpDecrementHandleCount(&ObjectHeader->Body, PsGetCurrentProcess(), - NewEntry.GrantedAccess); + GrantedAccess); + + /* Handle extra references */ + if (AdditionalReferences == 1) + { + /* Dereference the object once */ + ObDereferenceObject(Object); + } + else if (AdditionalReferences > 1) + { + /* Dereference it many times */ + InterlockedExchangeAdd(&ObjectHeader->PointerCount, + -AdditionalReferences); + } + + /* Detach if necessary and fail */ + if (AttachedToProcess) KeUnstackDetachProcess(&ApcState); return STATUS_INSUFFICIENT_RESOURCES; }
Modified: trunk/reactos/ntoskrnl/ob/obname.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obname.c?rev=25... ============================================================================== --- trunk/reactos/ntoskrnl/ob/obname.c (original) +++ trunk/reactos/ntoskrnl/ob/obname.c Tue Jan 9 20:18:22 2007 @@ -292,19 +292,19 @@ IN POBP_LOOKUP_CONTEXT LookupContext, OUT PVOID *FoundObject) { - PVOID RootDirectory; - PVOID Directory = NULL, ParentDirectory = NULL; PVOID Object; POBJECT_HEADER ObjectHeader; + UNICODE_STRING ComponentName, RemainingName; + BOOLEAN InsideRoot = FALSE; + PDEVICE_MAP DeviceMap = NULL; + POBJECT_DIRECTORY Directory = NULL, ParentDirectory = NULL, RootDirectory; + POBJECT_DIRECTORY ReferencedDirectory = NULL, ReferencedParentDirectory = NULL; + KIRQL CalloutIrql; + OB_PARSE_METHOD ParseRoutine; NTSTATUS Status; + KPROCESSOR_MODE AccessCheckMode; PWCHAR NewName; POBJECT_HEADER_NAME_INFO ObjectNameInfo; - UNICODE_STRING RemainingName, ComponentName; - BOOLEAN InsideRoot = FALSE; - KPROCESSOR_MODE AccessCheckMode; - OB_PARSE_METHOD ParseRoutine; - KIRQL CalloutIrql; - POBJECT_DIRECTORY ReferencedDirectory = NULL, ReferencedParentDirectory = NULL; PAGED_CODE(); OBTRACE(OB_NAMESPACE_DEBUG, "%s - Finding Object: %wZ. Expecting: %p\n", @@ -337,7 +337,7 @@ 0, NULL, AccessMode, - &RootDirectory, + (PVOID*)&RootDirectory, NULL); if (!NT_SUCCESS(Status)) return Status;
@@ -377,7 +377,7 @@ Status = ParseRoutine(RootDirectory, ObjectType, AccessState, - AccessMode, + AccessCheckMode, Attributes, ObjectName, &RemainingName, @@ -725,7 +725,7 @@ Status = ParseRoutine(Object, ObjectType, AccessState, - AccessMode, + AccessCheckMode, Attributes, ObjectName, &RemainingName, @@ -873,7 +873,7 @@ }
/* Check if we have a device map and dereference it if so */ - //if (DeviceMap) ObfDereferenceDeviceMap(DeviceMap); + if (DeviceMap) ObfDereferenceDeviceMap(DeviceMap);
/* Check if we have a referenced directory and dereference it if so */ if (ReferencedDirectory) ObDereferenceObject(ReferencedDirectory);