[ros-diffs] [ion] 22685: - 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 r

Author: ion
Date: Thu Jun 29 09:05:27 2006
New Revision: 22685

URL: http://svn.reactos.org/svn/reactos?rev=22685&view=rev
Log:
- 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.

Modified:
    trunk/reactos/ntoskrnl/include/internal/ob.h
    trunk/reactos/ntoskrnl/io/controller.c
    trunk/reactos/ntoskrnl/io/device.c
    trunk/reactos/ntoskrnl/ob/obdir.c
    trunk/reactos/ntoskrnl/ob/obhandle.c
    trunk/reactos/ntoskrnl/ob/oblife.c
    trunk/reactos/ntoskrnl/ob/obref.c
    trunk/reactos/ntoskrnl/ob/obsecure.c
    trunk/reactos/ntoskrnl/ps/process.c

Modified: trunk/reactos/ntoskrnl/include/internal/ob.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ob.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/ob.h Thu Jun 29 09:05:27 2006
@@ -214,7 +214,7 @@
 );
 
 //
-// Security descriptor cache functions
+// Security functions
 //
 NTSTATUS
 NTAPI
@@ -245,6 +245,16 @@
 NTAPI
 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
 );
 
 //

Modified: trunk/reactos/ntoskrnl/io/controller.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/controller.c?r…
==============================================================================
--- trunk/reactos/ntoskrnl/io/controller.c (original)
+++ trunk/reactos/ntoskrnl/io/controller.c Thu Jun 29 09:05:27 2006
@@ -100,8 +100,8 @@
     Status = ObInsertObject(Controller,
                             NULL,
                             FILE_READ_DATA | FILE_WRITE_DATA,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&Controller,
                             &Handle);
    if (!NT_SUCCESS(Status)) return NULL;
 

Modified: trunk/reactos/ntoskrnl/io/device.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/device.c?rev=2…
==============================================================================
--- trunk/reactos/ntoskrnl/io/device.c (original)
+++ trunk/reactos/ntoskrnl/io/device.c Thu Jun 29 09:05:27 2006
@@ -566,8 +566,8 @@
     Status = ObInsertObject(CreatedDeviceObject,
                             NULL,
                             FILE_READ_DATA | FILE_WRITE_DATA,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&CreatedDeviceObject,
                             &TempHandle);
 
     if (!NT_SUCCESS(Status))

Modified: trunk/reactos/ntoskrnl/ob/obdir.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obdir.c?rev=22…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obdir.c (original)
+++ trunk/reactos/ntoskrnl/ob/obdir.c Thu Jun 29 09:05:27 2006
@@ -455,7 +455,7 @@
                                        PreviousMode,
                                        (PVOID*)&Directory,
                                        NULL);
-    if(NT_SUCCESS(Status))
+    if (NT_SUCCESS(Status))
     {
         /* FIXME: TODO. UNIMPLEMENTED */
         Status = STATUS_INSUFFICIENT_RESOURCES;

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 Thu Jun 29 09:05:27 2006
@@ -237,7 +237,17 @@
     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 @@
     /* 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 @@
     /* 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 @@
         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,

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 Thu Jun 29 09:05:27 2006
@@ -946,10 +946,32 @@
     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 @@
 
         /* 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;
 }
 

Modified: trunk/reactos/ntoskrnl/ob/obref.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obref.c?rev=22…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obref.c (original)
+++ trunk/reactos/ntoskrnl/ob/obref.c Thu Jun 29 09:05:27 2006
@@ -88,11 +88,30 @@
     /* 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)

Modified: trunk/reactos/ntoskrnl/ob/obsecure.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obsecure.c?rev…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obsecure.c (original)
+++ trunk/reactos/ntoskrnl/ob/obsecure.c Thu Jun 29 09:05:27 2006
@@ -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

Modified: trunk/reactos/ntoskrnl/ps/process.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/process.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/process.c (original)
+++ trunk/reactos/ntoskrnl/ps/process.c Thu Jun 29 09:05:27 2006
@@ -423,8 +423,8 @@
     Status = ObInsertObject(Process,
                             NULL,
                             DesiredAccess,
-                            0,
-                            NULL,
+                            1,
+                            (PVOID*)&Process,
                             &hProcess);
     if (NT_SUCCESS(Status))
     {