[ros-diffs] [ion] 25400: - 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.

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=254…
==============================================================================
--- 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?…
==============================================================================
--- 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=25…
==============================================================================
--- 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=…
==============================================================================
--- 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=2…
==============================================================================
--- 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);