[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/…
==============================================================================
--- 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=2…
==============================================================================
--- 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 */