- ObReferenceObjectByHandle/ObpReferenceProcessByHandle: Properly

return STATUS_INVALID_HANDLE if user-mode tries to reference a
kernel-mode handle.
- ObReferenceObjectByHandle: Properly validate process/thread access
rights before giving a reference to the caller.
- Fix definition of "SizeOfHandle" macro in the handle table
implementation. Fixes handle leaks at process rundown, handle
allocation, and problems with processes that use more than 512
handles.
- Remove checks for "VALID_INHERIT_FLAGS". These flags have nothing to
do with handle table entries and shouldn't appear in them. Please fix
callers if they're attempting to send inherit flags as access masks.
- Thanks to Alex! :)

svn path=/trunk/; revision=29304

diff --git a/reactos/ntoskrnl/ex/handle.c b/reactos/ntoskrnl/ex/handle.c
index 5c6b2f2..045de7c 100644 (file)
--- a/reactos/ntoskrnl/ex/handle.c
+++ b/reactos/ntoskrnl/ex/handle.c
@@ -17,7 +17,7 @@
 
 LIST_ENTRY HandleTableListHead;
 EX_PUSH_LOCK HandleTableListLock;
-#define SizeOfHandle(x) sizeof(HANDLE) * x
+#define SizeOfHandle(x) (sizeof(HANDLE) * x)
 
 /* PRIVATE FUNCTIONS *********************************************************/
 

diff --git a/reactos/ntoskrnl/ob/obhandle.c b/reactos/ntoskrnl/ob/obhandle.c
index 2f5458e..4cb5eda 100644 (file)
--- a/reactos/ntoskrnl/ob/obhandle.c
+++ b/reactos/ntoskrnl/ob/obhandle.c
@@ -73,55 +73,64 @@ ObpReferenceProcessObjectByHandle(IN HANDLE Handle,
 
     /* Assume failure */
     *Object = NULL;
-
-    /* Check if the caller wants the current process */
-    if (Handle == NtCurrentProcess())
-    {
-        /* Return handle info */
-        HandleInformation->HandleAttributes = 0;
-        HandleInformation->GrantedAccess = Process->GrantedAccess;
-
-        /* No audit mask */
-        *AuditMask = 0;
-
-        /* Reference ourselves */
-        ObjectHeader = OBJECT_TO_OBJECT_HEADER(Process);
-        InterlockedIncrement(&ObjectHeader->PointerCount);
-
-        /* Return the pointer */
-        *Object = Process;
-        ASSERT(*Object != NULL);
-        return STATUS_SUCCESS;
-    }
-
-    /* Check if the caller wants the current thread */
-    if (Handle == NtCurrentThread())
-    {
-        /* Return handle information */
-        HandleInformation->HandleAttributes = 0;
-        HandleInformation->GrantedAccess = Thread->GrantedAccess;
-
-        /* Reference ourselves */
-        ObjectHeader = OBJECT_TO_OBJECT_HEADER(Thread);
-        InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
-
-        /* No audit mask */
-        *AuditMask = 0;
-
-        /* Return the pointer */
-        *Object = Thread;
-        ASSERT(*Object != NULL);
-        return STATUS_SUCCESS;
-    }
-
-    /* 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;
+    
+    /* Check if this is a special handle */
+    if (HandleToLong(Handle) < 0)
+    {
+        /* Check if the caller wants the current process */
+        if (Handle == NtCurrentProcess())
+        {
+            /* Return handle info */
+            HandleInformation->HandleAttributes = 0;
+            HandleInformation->GrantedAccess = Process->GrantedAccess;
+            
+            /* No audit mask */
+            *AuditMask = 0;
+            
+            /* Reference ourselves */
+            ObjectHeader = OBJECT_TO_OBJECT_HEADER(Process);
+            InterlockedIncrement(&ObjectHeader->PointerCount);
+            
+            /* Return the pointer */
+            *Object = Process;
+            ASSERT(*Object != NULL);
+            return STATUS_SUCCESS;
+        }
+        
+        /* Check if the caller wants the current thread */
+        if (Handle == NtCurrentThread())
+        {
+            /* Return handle information */
+            HandleInformation->HandleAttributes = 0;
+            HandleInformation->GrantedAccess = Thread->GrantedAccess;
+            
+            /* Reference ourselves */
+            ObjectHeader = OBJECT_TO_OBJECT_HEADER(Thread);
+            InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
+            
+            /* No audit mask */
+            *AuditMask = 0;
+            
+            /* Return the pointer */
+            *Object = Thread;
+            ASSERT(*Object != NULL);
+            return STATUS_SUCCESS;
+        }
+        
+        /* This is a kernel handle... do we have access? */
+        if (AccessMode == KernelMode)
+        {
+            /* Use the kernel handle table and get the actual handle value */
+            Handle = ObKernelHandleToHandle(Handle);
+            HandleTable = ObpKernelHandleTable;
+        }
+        else
+        {
+            /* This is an illegal attempt to access a kernel handle */
+            return STATUS_INVALID_HANDLE;
+        }
     }
-
+        
     /* Enter a critical region while we touch the handle table */
     ASSERT(HandleTable != NULL);
     KeEnterCriticalRegion();
@@ -2053,12 +2062,7 @@ ObKillProcess(IN PEPROCESS Process)
     ExSweepHandleTable(HandleTable,
                        ObpCloseHandleCallback,
                        &Context);
-    //ASSERT(HandleTable->HandleCount == 0);
-    /* HACK: Until the problem is investigated... */
-    if (HandleTable->HandleCount != 0)
-    {
-        DPRINT1("Leaking %d handles!\n", HandleTable->HandleCount);
-    }
+    ASSERT(HandleTable->HandleCount == 0);
 
     /* Leave the critical region */
     KeLeaveCriticalRegion();

diff --git a/reactos/ntoskrnl/ob/obref.c b/reactos/ntoskrnl/ob/obref.c
index 8c50acc..87e33d5 100644 (file)
--- a/reactos/ntoskrnl/ob/obref.c
+++ b/reactos/ntoskrnl/ob/obref.c
@@ -480,70 +480,111 @@ ObReferenceObjectByHandle(IN HANDLE Handle,
     /* Assume failure */
     *Object = NULL;
 
-    /* Check if the caller wants the current process */
-    if ((Handle == NtCurrentProcess()) &&
-        ((ObjectType == PsProcessType) || !(ObjectType)))
+    /* Check if this is a special handle */
+    if (HandleToLong(Handle) < 0)
     {
-        /* Get the current process */
-        CurrentProcess = PsGetCurrentProcess();
-
-        /* Check if the caller wanted handle information */
-        if (HandleInformation)
+        /* Check if this is the current process */
+        if (Handle == NtCurrentProcess())
         {
-            /* Return it */
-            HandleInformation->HandleAttributes = 0;
-            HandleInformation->GrantedAccess = PROCESS_ALL_ACCESS;
+            /* Check if this is the right object type */
+            if ((ObjectType == PsProcessType) || !(ObjectType))
+            {
+                /* Get the current process and granted access */
+                CurrentProcess = PsGetCurrentProcess();
+                GrantedAccess = CurrentProcess->GrantedAccess;
+                
+                /* Validate access */
+                if ((AccessMode == KernelMode) ||
+                    !(~GrantedAccess & DesiredAccess))
+                {                   
+                    /* Check if the caller wanted handle information */
+                    if (HandleInformation)
+                    {
+                        /* Return it */
+                        HandleInformation->HandleAttributes = 0;
+                        HandleInformation->GrantedAccess = GrantedAccess;
+                    }
+                    
+                    /* Reference ourselves */
+                    ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentProcess);
+                    InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
+                    
+                    /* Return the pointer */
+                    *Object = CurrentProcess;
+                    ASSERT(*Object != NULL);
+                    Status = STATUS_SUCCESS;
+                }
+                else
+                {
+                    /* Access denied */
+                    Status = STATUS_ACCESS_DENIED;
+                }
+            }
+            else
+            {
+                /* The caller used this special handle value with a non-process type */
+                Status = STATUS_OBJECT_TYPE_MISMATCH;
+            }
+            
+            /* Return the status */
+            return Status;
         }
-
-        /* Reference ourselves */
-        ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentProcess);
-        InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
-
-        /* Return the pointer */
-        *Object = CurrentProcess;
-        return STATUS_SUCCESS;
-    }
-    else if (Handle == NtCurrentProcess())
-    {
-        /* 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();
-
-        /* Check if the caller wanted handle information */
-        if (HandleInformation)
+        else if (Handle == NtCurrentThread())
         {
-            /* Return it */
-            HandleInformation->HandleAttributes = 0;
-            HandleInformation->GrantedAccess = THREAD_ALL_ACCESS;
+            /* Check if this is the right object type */
+            if ((ObjectType == PsThreadType) || !(ObjectType))
+            {
+                /* Get the current process and granted access */
+                CurrentThread = PsGetCurrentThread();
+                GrantedAccess = CurrentThread->GrantedAccess;
+                
+                /* Validate access */
+                if ((AccessMode == KernelMode) ||
+                    !(~GrantedAccess & DesiredAccess))
+                {                   
+                    /* Check if the caller wanted handle information */
+                    if (HandleInformation)
+                    {
+                        /* Return it */
+                        HandleInformation->HandleAttributes = 0;
+                        HandleInformation->GrantedAccess = GrantedAccess;
+                    }
+                    
+                    /* Reference ourselves */
+                    ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentThread);
+                    InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
+                    
+                    /* Return the pointer */
+                    *Object = CurrentThread;
+                    ASSERT(*Object != NULL);
+                    Status = STATUS_SUCCESS;
+                }
+                else
+                {
+                    /* Access denied */
+                    Status = STATUS_ACCESS_DENIED;
+                }
+            }
+            else
+            {
+                /* The caller used this special handle value with a non-process type */
+                Status = STATUS_OBJECT_TYPE_MISMATCH;
+            }
+            
+            /* Return the status */
+            return Status;
+        }
+        else if (AccessMode == KernelMode)
+        {
+            /* Use the kernel handle table and get the actual handle value */
+            Handle = ObKernelHandleToHandle(Handle);
+            HandleTable = ObpKernelHandleTable;
+        }
+        else
+        {
+            /* Invalid access, fail */
+            return STATUS_INVALID_HANDLE;
         }
-
-        /* Reference ourselves */
-        ObjectHeader = OBJECT_TO_OBJECT_HEADER(CurrentThread);
-        InterlockedExchangeAdd(&ObjectHeader->PointerCount, 1);
-
-        /* Return the pointer */
-        *Object = CurrentThread;
-        return STATUS_SUCCESS;
-    }
-    else if (Handle == NtCurrentThread())
-    {
-        /* 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
     {
@@ -565,11 +606,10 @@ ObReferenceObjectByHandle(IN HANDLE Handle,
         {
             /* Get the granted access and validate it */
             GrantedAccess = HandleEntry->GrantedAccess;
-            /* Inherit flags are not a kind of access right, they indicate 
-             * the disposition of access rights.  Therefore, they should not
-             * be considered. */
+            
+            /* Validate access */
             if ((AccessMode == KernelMode) ||
-                !((~GrantedAccess & DesiredAccess & ~VALID_INHERIT_FLAGS)))
+                !(~GrantedAccess & DesiredAccess))
             {
                 /* Reference the object directly since we have its header */
                 InterlockedIncrement(&ObjectHeader->PointerCount);