- This patch finally enables closing handles for real, when NtClose is called. This means that File handles, processes, and all other NT Objects finally die (no more "file in use" and unkillable processes). On the other hand, this makes the registry code unhappy and unravelled a bug in ObDuplicateObject.

- Booting/installing still works, but the system will possibly be unstable. However I'm choosing to commit this because it shows correct Ob behavior and will allow Art to fix Cm's referencing properly.
- Implement ObCheckObjectAccess and call it to perform access verification checks.
- Properly create devices, processes and controllers with an extra reference count, so that when the code closes their handle they don't die.
- Check for invalid uses of ObfDereferenceObject for our current debugging purposes.
- Add SEH to NtQueryObject.

svn path=/trunk/; revision=22685

reactos/ntoskrnl/include/internal/ob.h

@@ -214,7 +214,7 @@ ObpDeleteNameCheck(
 );
 
 //
-// Security descriptor cache functions
+// Security functions
 //
 NTSTATUS
 NTAPI
@@ -247,6 +247,16 @@ ObpDereferenceCachedSecurityDescriptor(
     IN PSECURITY_DESCRIPTOR SecurityDescriptor
 );
 
+BOOLEAN
+NTAPI
+ObCheckObjectAccess(
+    IN PVOID Object,
+    IN OUT PACCESS_STATE AccessState,
+    IN BOOLEAN Unknown,
+    IN KPROCESSOR_MODE AccessMode,
+    OUT PNTSTATUS ReturnedStatus
+);
+
 //
 // Executive Fast Referencing Functions
 //

reactos/ntoskrnl/io/controller.c

@@ -100,8 +100,8 @@ IoCreateController(ULONG Size)
     Status = ObInsertObject(Controller,
                             NULL,
                             FILE_READ_DATA | FILE_WRITE_DATA,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&Controller,
                             &Handle);
    if (!NT_SUCCESS(Status)) return NULL;
 

reactos/ntoskrnl/io/device.c

@@ -566,8 +566,8 @@ IoCreateDevice(PDRIVER_OBJECT DriverObject,
     Status = ObInsertObject(CreatedDeviceObject,
                             NULL,
                             FILE_READ_DATA | FILE_WRITE_DATA,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&CreatedDeviceObject,
                             &TempHandle);
 
     if (!NT_SUCCESS(Status))

reactos/ntoskrnl/ob/obdir.c

@@ -455,7 +455,7 @@ NtQueryDirectoryObject(IN HANDLE DirectoryHandle,
                                        PreviousMode,
                                        (PVOID*)&Directory,
                                        NULL);
-    if(NT_SUCCESS(Status))
+    if (NT_SUCCESS(Status))
     {
         /* FIXME: TODO. UNIMPLEMENTED */
         Status = STATUS_INSUFFICIENT_RESOURCES;

reactos/ntoskrnl/ob/obhandle.c

@@ -237,7 +237,17 @@ ObpCloseHandleTableEntry(IN PHANDLE_TABLE HandleTable,
     ObpDecrementHandleCount(Body, PsGetCurrentProcess(), GrantedAccess);
 
     /* Dereference the object as well */
-    //ObDereferenceObject(Body); // FIXME: Needs sync changes in other code
+    if (!wcscmp(ObjectHeader->Type->Name.Buffer, L"Key"))
+    {
+        //
+        // WE DONT CLOSE REGISTRY HANDLES BECAUSE CM IS BRAINDEAD
+        //
+        DPRINT("NOT CLOSING THE KEY\n");
+    }
+    else
+    {
+        ObDereferenceObject(Body);
+    }
 
     /* Return to caller */
     OBTRACE(OB_HANDLE_DEBUG,
@@ -310,20 +320,16 @@ ObpIncrementHandleCount(IN PVOID Object,
     /* Check if we're opening an existing handle */
     if (OpenReason == ObOpenHandle)
     {
-        /*
-         * FIXME: Do validation as described in Chapter 8
-         * of Windows Internals 4th.
-         */
-#if 0
+        /* Validate the caller's access to this object */
         if (!ObCheckObjectAccess(Object,
                                  AccessState,
                                  TRUE,
                                  AccessMode,
                                  &Status))
         {
+            /* Access was denied, so fail */
             return Status;
         }
-#endif
     }
     else if (OpenReason == ObCreateHandle)
     {
@@ -982,7 +988,14 @@ ObpSetHandleAttributes(IN PHANDLE_TABLE HandleTable,
     /* Check if making the handle inheritable */
     if (SetHandleInfo->Information.Inherit)
     {
-        /* Set the flag. FIXME: Need to check if this is allowed */
+        /* Check if inheriting is not supported for this object */
+        if (ObjectHeader->Type->TypeInfo.InvalidAttributes & OBJ_INHERIT)
+        {
+            /* Fail without changing anything */
+            return FALSE;
+        }
+
+        /* Set the flag */
         HandleTableEntry->ObAttributes |= EX_HANDLE_ENTRY_INHERITABLE;
     }
     else
@@ -1757,7 +1770,7 @@ ObInsertObject(IN PVOID Object,
         Header->ObjectCreateInfo = NULL;
 
         /* Remove the extra keep-alive reference */
-        if (Handle) ObDereferenceObject(Object); // FIXME: Needs sync changes
+        if (Handle) ObDereferenceObject(Object);
 
         /* Return */
         OBTRACE(OB_HANDLE_DEBUG,

reactos/ntoskrnl/ob/oblife.c

@@ -946,10 +946,32 @@ NtQueryObject(IN HANDLE ObjectHandle,
     POBJECT_BASIC_INFORMATION BasicInfo;
     ULONG InfoLength;
     PVOID Object = NULL;
-    NTSTATUS Status;
+    NTSTATUS Status = STATUS_SUCCESS;
+    KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
     PAGED_CODE();
 
-    /* FIXME: Needs SEH */
+    /* Check if the caller is from user mode */
+    if (PreviousMode != KernelMode)
+    {
+        /* Protect validation with SEH */
+        _SEH_TRY
+        {
+            /* Probe the input structure */
+            ProbeForWrite(ObjectInformation, Length, sizeof(UCHAR));
+
+            /* If we have a result length, probe it too */
+            if (ResultLength) ProbeForWriteUlong(ResultLength);
+        }
+        _SEH_HANDLE
+        {
+            /* Get the exception code */
+            Status = _SEH_GetExceptionCode();
+        }
+        _SEH_END;
+
+        /* Fail if we raised an exception */
+        if (!NT_SUCCESS(Status)) return Status;
+    }
 
     /*
      * Make sure this isn't a generic type query, since the caller doesn't
@@ -1082,16 +1104,33 @@ NtQueryObject(IN HANDLE ObjectHandle,
 
         /* Anything else */
         default:
+
             /* Fail it */
             Status = STATUS_INVALID_INFO_CLASS;
             break;
     }
 
-    /* Derefernece the object if we had referenced it */
+    /* Dereference the object if we had referenced it */
     if (Object) ObDereferenceObject (Object);
 
-    /* Return the length and status */
-    if (ResultLength) *ResultLength = InfoLength;
+    /* Check if the caller wanted the return length */
+    if (ResultLength)
+    {
+        /* Protect the write to user mode */
+        _SEH_TRY
+        {
+            /* Write the length */
+            *ResultLength = Length;
+        }
+        _SEH_HANDLE
+        {
+            /* Otherwise, get the exception code */
+            Status = _SEH_GetExceptionCode();
+        }
+        _SEH_END;
+    }
+
+    /* Return status */
     return Status;
 }
 

reactos/ntoskrnl/ob/obref.c

@@ -88,11 +88,30 @@ ObfDereferenceObject(IN PVOID Object)
     /* Extract the object header */
     Header = OBJECT_TO_OBJECT_HEADER(Object);
 
+    if (Header->PointerCount <= Header->HandleCount)
+    {
+        if (!wcscmp(Header->Type->Name.Buffer, L"Event"))
+        {
+            //KEBUGCHECK(0);
+        }
+        DPRINT1("Misbehaving object: %wZ\n", &Header->Type->Name);
+    }
+
     /* Check whether the object can now be deleted. */
     if (!(InterlockedDecrement(&Header->PointerCount)))
     {
         /* Sanity check */
-        ASSERT(!Header->HandleCount);
+        if (wcscmp(Header->Type->Name.Buffer, L"Key"))
+        {
+            if(Header->HandleCount)
+            {
+                DPRINT1("Unexpected misbehaving object: %wZ\n", &Header->Type->Name);
+            }
+        }
+        else
+        {
+            DPRINT1("Cm needs fixing!\n");
+        }
 
         /* Check if we're at PASSIVE */
         if (KeGetCurrentIrql() == PASSIVE_LEVEL)

reactos/ntoskrnl/ob/obsecure.c

@@ -15,7 +15,107 @@
 
 #define TAG_SEC_QUERY   TAG('O', 'b', 'S', 'q')
 
-/* FUNCTIONS ***************************************************************/
+/* PRIVATE FUNCTIONS *********************************************************/
+
+/*++
+* @name ObCheckObjectAccess
+*
+*     The ObAssignSecurity routine <FILLMEIN>
+*
+* @param Object
+*        <FILLMEIN>
+*
+* @param AccessState
+*        <FILLMEIN>
+*
+* @param Unknown
+*        <FILLMEIN>
+*
+* @param AccessMode
+*        <FILLMEIN>
+*
+* @param ReturnedStatus
+*        <FILLMEIN>
+*
+* @return TRUE if access was granted, FALSE otherwise.
+*
+* @remarks None.
+*
+*--*/
+BOOLEAN
+NTAPI
+ObCheckObjectAccess(IN PVOID Object,
+                    IN OUT PACCESS_STATE AccessState,
+                    IN BOOLEAN Unknown,
+                    IN KPROCESSOR_MODE AccessMode,
+                    OUT PNTSTATUS ReturnedStatus)
+{
+    POBJECT_HEADER ObjectHeader;
+    POBJECT_TYPE ObjectType;
+    PSECURITY_DESCRIPTOR SecurityDescriptor = NULL;
+    BOOLEAN SdAllocated;
+    NTSTATUS Status;
+    BOOLEAN Result;
+    ACCESS_MASK GrantedAccess;
+    PPRIVILEGE_SET Privileges = NULL;
+    PAGED_CODE();
+
+    /* Get the object header and type */
+    ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object);
+    ObjectType = ObjectHeader->Type;
+
+    /* Get security information */
+    Status = ObGetObjectSecurity(Object, &SecurityDescriptor, &SdAllocated);
+    if (!NT_SUCCESS(Status))
+    {
+        /* Return failure */
+        *ReturnedStatus = Status;
+        return FALSE;
+    }
+    else if (!SecurityDescriptor)
+    {
+        /* Otherwise, if we don't actually have an SD, return success */
+        *ReturnedStatus = Status;
+        return TRUE;
+    }
+
+    /* Lock the security context */
+    SeLockSubjectContext(&AccessState->SubjectSecurityContext);
+
+    /* Now do the entire access check */
+    Result = SeAccessCheck(SecurityDescriptor,
+                           &AccessState->SubjectSecurityContext,
+                           TRUE,
+                           AccessState->RemainingDesiredAccess,
+                           AccessState->PreviouslyGrantedAccess,
+                           &Privileges,
+                           &ObjectType->TypeInfo.GenericMapping,
+                           AccessMode,
+                           &GrantedAccess,
+                           ReturnedStatus);
+    if (Privileges)
+    {
+        /* We got privileges, append them to teh access state and free them */
+        Status = SeAppendPrivileges(AccessState, Privileges);
+        SeFreePrivileges(Privileges);
+    }
+
+    /* Check if access was granted */
+    if (Result)
+    {
+        /* Update the access state */
+        AccessState->RemainingDesiredAccess &= ~(GrantedAccess |
+                                                 MAXIMUM_ALLOWED);
+        AccessState->PreviouslyGrantedAccess |= GrantedAccess;
+    }
+
+    /* We're done, unlock the context and release security */
+    SeUnlockSubjectContext(&AccessState->SubjectSecurityContext);
+    ObReleaseObjectSecurity(SecurityDescriptor, SdAllocated);
+    return Result;
+}
+
+/* PUBLIC FUNCTIONS **********************************************************/
 
 /*++
 * @name ObAssignSecurity

reactos/ntoskrnl/ps/process.c

@@ -423,8 +423,8 @@ PspCreateProcess(OUT PHANDLE ProcessHandle,
     Status = ObInsertObject(Process,
                             NULL,
                             DesiredAccess,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&Process,
                             &hProcess);
     if (NT_SUCCESS(Status))
     {