[ros-diffs] [ion] 22231: - Fixed formatting/commented/annotated ObReferenceObjectByHandle. - Bug fixes: * Remove MAXIMUM_ALLOWED<->GENERIC_ALL conversion, I could find no mention of this in the docs. * Remove GENERIC_ACCESS <-> RtlMapGenericMask conversion, I could find no mention of this in the docs, and this mapping is only required when creating handles, not when referencing pointers. - Optimizations: * Restructure code and remove code which was sometimes duplicated up to 5 times. * Do not attach/detach

Author: ion
Date: Mon Jun  5 09:07:44 2006
New Revision: 22231

URL: http://svn.reactos.ru/svn/reactos?rev=22231&view=rev
Log:
- Fixed formatting/commented/annotated ObReferenceObjectByHandle.
- Bug fixes:
  * Remove MAXIMUM_ALLOWED<->GENERIC_ALL conversion, I could find no mention of this
in the docs.
  * Remove GENERIC_ACCESS <-> RtlMapGenericMask conversion, I could find no mention
of this in the docs, and this mapping is only required when creating handles, not when
referencing pointers.
- Optimizations:
  * Restructure code and remove code which was sometimes duplicated up to 5 times.
  * Do not attach/detach from the system process, this isn't required since we're
merely getting a kernel pointer from the handle netry.
  * Directly increase the pointer count instead of calling ObReferenceObject, since we
already have the object header in a variable.
  * Cache ObpKernelHandleTable/Process->ObjectTable and use those directly instead of
always de-referencing the process.

Modified:
    trunk/reactos/base/system/smss/client.c
    trunk/reactos/ntoskrnl/ob/obref.c

Modified: trunk/reactos/base/system/smss/client.c
URL:
http://svn.reactos.ru/svn/reactos/trunk/reactos/base/system/smss/client.c?r…
==============================================================================
--- trunk/reactos/base/system/smss/client.c (original)
+++ trunk/reactos/base/system/smss/client.c Mon Jun  5 09:07:44 2006
@@ -194,6 +194,14 @@
 			SmpClientDirectory.CandidateClient->ServerProcessId =
 				(ULONG) pbi.UniqueProcessId;
 		}
+        else
+        {
+            LARGE_INTEGER Fixme;
+            Fixme.QuadPart = -50000000;
+            DPRINT1("WARNING! UniqueProcess IS A THREAD HANDLE!!!\n");
+            NtDelayExecution(FALSE, &Fixme);
+            DPRINT1("FIXME!\n");
+        }
 	}
 	else
 	{

Modified: trunk/reactos/ntoskrnl/ob/obref.c
URL:
http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obref.c?rev=222…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obref.c (original)
+++ trunk/reactos/ntoskrnl/ob/obref.c Mon Jun  5 09:07:44 2006
@@ -127,6 +127,18 @@
             ExQueueWorkItem(&ObpReaperWorkItem, DelayedWorkQueue);
         }
     }
+}
+
+#ifdef ObDereferenceObject
+#undef ObDereferenceObject
+#endif
+
+VOID
+NTAPI
+ObDereferenceObject(IN PVOID Object)
+{
+    /* Call the fastcall function */
+    ObfDereferenceObject(Object);
 }
 
 NTSTATUS
@@ -228,206 +240,165 @@
     return Status;
 }
 
-NTSTATUS STDCALL
-ObReferenceObjectByHandle(HANDLE Handle,
-                          ACCESS_MASK DesiredAccess,
-                          POBJECT_TYPE ObjectType,
-                          KPROCESSOR_MODE AccessMode,
-                          PVOID* Object,
-                          POBJECT_HANDLE_INFORMATION HandleInformation)
+NTSTATUS
+NTAPI
+ObReferenceObjectByHandle(IN HANDLE Handle,
+                          IN ACCESS_MASK DesiredAccess,
+                          IN POBJECT_TYPE ObjectType,
+                          IN KPROCESSOR_MODE AccessMode,
+                          OUT PVOID* Object,
+                          OUT POBJECT_HANDLE_INFORMATION HandleInformation OPTIONAL)
 {
     PHANDLE_TABLE_ENTRY HandleEntry;
     POBJECT_HEADER ObjectHeader;
-    PVOID ObjectBody;
     ACCESS_MASK GrantedAccess;
     ULONG Attributes;
-    PEPROCESS CurrentProcess, Process;
-    BOOLEAN AttachedToProcess = FALSE;
-    KAPC_STATE ApcState;
-
+    PEPROCESS CurrentProcess;
+    PVOID HandleTable;
+    PETHREAD CurrentThread;
+    NTSTATUS Status;
     PAGED_CODE();
 
-    DPRINT("ObReferenceObjectByHandle(Handle %p, DesiredAccess %x, "
-        "ObjectType %p, AccessMode %d, Object %p)\n",Handle,DesiredAccess,
-        ObjectType,AccessMode,Object);
-
-    if (Handle == NULL)
-    {
-        return STATUS_INVALID_HANDLE;
-    }
-
-    CurrentProcess = PsGetCurrentProcess();
-
-    /*
-    * Handle special handle names
-    */
-    if (Handle == NtCurrentProcess() &&
-        (ObjectType == PsProcessType || ObjectType == NULL))
-    {
+    /* Fail immediately if the handle is NULL */
+    if (!Handle) return STATUS_INVALID_HANDLE;
+
+    /* Check if the caller wants the current process */
+    if ((Handle == NtCurrentProcess()) &&
+        ((ObjectType == PsProcessType) || !(ObjectType)))
+    {
+        /* Get the current process */
+        CurrentProcess = PsGetCurrentProcess();
+
+        /* Reference ourselves */
         ObReferenceObject(CurrentProcess);
 
-        if (HandleInformation != NULL)
-        {
+        /* Check if the caller wanted handle information */
+        if (HandleInformation)
+        {
+            /* Return it */
             HandleInformation->HandleAttributes = 0;
             HandleInformation->GrantedAccess = PROCESS_ALL_ACCESS;
         }
 
+        /* Return the pointer */
         *Object = CurrentProcess;
-        DPRINT("Referencing current process %p\n", CurrentProcess);
         return STATUS_SUCCESS;
     }
     else if (Handle == NtCurrentProcess())
     {
-        CHECKPOINT;
-        return(STATUS_OBJECT_TYPE_MISMATCH);
-    }
-
-    if (Handle == NtCurrentThread() &&
-        (ObjectType == PsThreadType || ObjectType == NULL))
-    {
-        PETHREAD CurrentThread = PsGetCurrentThread();
-
+        /* The caller used this special handle value with a non-process type */
+        return STATUS_OBJECT_TYPE_MISMATCH;
+    }
+
+    /* Check if the caller wants the current thread */
+    if ((Handle == NtCurrentThread()) &&
+        ((ObjectType == PsThreadType) || !(ObjectType)))
+    {
+        /* Get the current thread */
+        CurrentThread = PsGetCurrentThread();
+
+        /* Reference ourselves */
         ObReferenceObject(CurrentThread);
 
-        if (HandleInformation != NULL)
-        {
+        /* Check if the caller wanted handle information */
+        if (HandleInformation)
+        {
+            /* Return it */
             HandleInformation->HandleAttributes = 0;
             HandleInformation->GrantedAccess = THREAD_ALL_ACCESS;
         }
 
+        /* Return the pointer */
         *Object = CurrentThread;
-        CHECKPOINT;
         return STATUS_SUCCESS;
     }
     else if (Handle == NtCurrentThread())
     {
-        CHECKPOINT;
-        return(STATUS_OBJECT_TYPE_MISMATCH);
-    }
-
-    /* desire as much access rights as possible */
-    if (DesiredAccess & MAXIMUM_ALLOWED)
-    {
-        DesiredAccess &= ~MAXIMUM_ALLOWED;
-        DesiredAccess |= GENERIC_ALL;
-    }
-
-    if(ObIsKernelHandle(Handle, AccessMode))
-    {
-        Process = PsInitialSystemProcess;
+        /* The caller used this special handle value with a non-thread type */
+        return STATUS_OBJECT_TYPE_MISMATCH;
+    }
+
+    /* 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
     {
-        Process = CurrentProcess;
-    }
-
+        /* Otherwise use this process's handle table */
+        HandleTable = PsGetCurrentProcess()->ObjectTable;
+    }
+
+    /* Enter a critical region while we touch the handle table */
     KeEnterCriticalRegion();
 
-    if (Process != CurrentProcess)
-    {
-        KeStackAttachProcess(&Process->Pcb,
-            &ApcState);
-        AttachedToProcess = TRUE;
-    }
-
-    HandleEntry = ExMapHandleToPointer(Process->ObjectTable,
-        Handle);
-    if (HandleEntry == NULL)
-    {
-        if (AttachedToProcess)
-        {
-            KeUnstackDetachProcess(&ApcState);
-        }
-        KeLeaveCriticalRegion();
-        DPRINT("ExMapHandleToPointer() failed for handle 0x%p\n", Handle);
-        return(STATUS_INVALID_HANDLE);
-    }
-
-    ObjectHeader = EX_HTE_TO_HDR(HandleEntry);
-    ObjectBody = &ObjectHeader->Body;
-
-    DPRINT("locked1: ObjectHeader: 0x%p [HT:0x%p]\n", ObjectHeader,
Process->ObjectTable);
-
-    if (ObjectType != NULL && ObjectType != ObjectHeader->Type)
-    {
-        DPRINT("ObjectType mismatch: %wZ vs %wZ (handle 0x%p)\n",
&ObjectType->Name, ObjectHeader->Type ? &ObjectHeader->Type->Name :
NULL, Handle);
-
-        ExUnlockHandleTableEntry(Process->ObjectTable,
-            HandleEntry);
-
-        if (AttachedToProcess)
-        {
-            KeUnstackDetachProcess(&ApcState);
-        }
-
-        KeLeaveCriticalRegion();
-
-        return(STATUS_OBJECT_TYPE_MISMATCH);
-    }
-
-    /* map the generic access masks if the caller asks for generic access */
-    if (DesiredAccess & GENERIC_ACCESS)
-    {
-        RtlMapGenericMask(&DesiredAccess,
-           
&OBJECT_TO_OBJECT_HEADER(ObjectBody)->Type->TypeInfo.GenericMapping);
-    }
-
-    GrantedAccess = HandleEntry->GrantedAccess;
-
-    /* Unless running as KernelMode, deny access if caller desires more access
-    rights than the handle can grant */
-    if(AccessMode != KernelMode && (~GrantedAccess & DesiredAccess))
-    {
-        ExUnlockHandleTableEntry(Process->ObjectTable,
-            HandleEntry);
-
-        if (AttachedToProcess)
-        {
-            KeUnstackDetachProcess(&ApcState);
-        }
-
-        KeLeaveCriticalRegion();
-
-        DPRINT1("GrantedAccess: 0x%x, ~GrantedAccess: 0x%x, DesiredAccess: 0x%x,
denied: 0x%x\n", GrantedAccess, ~GrantedAccess, DesiredAccess, ~GrantedAccess &
DesiredAccess);
-
-        return(STATUS_ACCESS_DENIED);
-    }
-
-    ObReferenceObject(ObjectBody);
-
-    Attributes = HandleEntry->ObAttributes & (EX_HANDLE_ENTRY_PROTECTFROMCLOSE |
-        EX_HANDLE_ENTRY_INHERITABLE |
-        EX_HANDLE_ENTRY_AUDITONCLOSE);
-
-    ExUnlockHandleTableEntry(Process->ObjectTable,
-        HandleEntry);
-
-    if (AttachedToProcess)
-    {
-        KeUnstackDetachProcess(&ApcState);
-    }
-
+    /* Get the handle entry */
+    HandleEntry = ExMapHandleToPointer(HandleTable, Handle);
+    if (HandleEntry)
+    {
+        /* Get the object header and validate the type*/
+        ObjectHeader = EX_HTE_TO_HDR(HandleEntry);
+        if (!(ObjectType) || (ObjectType == ObjectHeader->Type))
+        {
+            /* Get the granted access and validate it */
+            GrantedAccess = HandleEntry->GrantedAccess;
+            if ((AccessMode == KernelMode) ||
+                !(~GrantedAccess & DesiredAccess))
+            {
+                /* Reference the object directly since we have its header */
+                InterlockedIncrement(&ObjectHeader->PointerCount);
+
+                /* Mask out the internal attributes */
+                Attributes = HandleEntry->ObAttributes &
+                             (EX_HANDLE_ENTRY_PROTECTFROMCLOSE |
+                              EX_HANDLE_ENTRY_INHERITABLE |
+                              EX_HANDLE_ENTRY_AUDITONCLOSE);
+
+                /* Check if the caller wants handle information */
+                if (HandleInformation)
+                {
+                    /* Fill out the information */
+                    HandleInformation->HandleAttributes = Attributes;
+                    HandleInformation->GrantedAccess = GrantedAccess;
+                }
+
+                /* Return the pointer */
+                *Object = &ObjectHeader->Body;
+
+                /* Unlock the handle */
+                ExUnlockHandleTableEntry(HandleTable, HandleEntry);
+
+                /* Return success */
+                KeLeaveCriticalRegion();
+                return STATUS_SUCCESS;
+            }
+            else
+            {
+                /* Requested access failed */
+                Status = STATUS_ACCESS_DENIED;
+            }
+        }
+        else
+        {
+            /* Invalid object type */
+            Status = STATUS_OBJECT_TYPE_MISMATCH;
+        }
+
+        /* Unlock the entry */
+        ExUnlockHandleTableEntry(HandleTable, HandleEntry);
+    }
+    else
+    {
+        /* Invalid handle */
+        Status = STATUS_INVALID_HANDLE;
+    }
+
+    /* Return failure status */
     KeLeaveCriticalRegion();
-
-    if (HandleInformation != NULL)
-    {
-        HandleInformation->HandleAttributes = Attributes;
-        HandleInformation->GrantedAccess = GrantedAccess;
-    }
-
-    *Object = ObjectBody;
-
-    return(STATUS_SUCCESS);
-}
-
-#ifdef ObDereferenceObject
-#undef ObDereferenceObject
-#endif
-
-VOID STDCALL
-ObDereferenceObject(IN PVOID Object)
-{
-    ObfDereferenceObject(Object);
-}
+    *Object = NULL;
+    return Status;
+}
+
 /* EOF */