[ros-diffs] [ion] 25394: - Enable sanity check in ObInsertObject to catch assholes that were calling it incorrectly (without a Handle output parameter, which is only allowed in a specific scenario). Changes: - Registry code which was calling ObInsertObject for no reason at all. Now an ugly hack has been added to Cm code to perform the only operation the insert did -> to free the create info. - SeSubProcessToken was broken and calling it incorrectly, fixed. - \Device\PhysicalMemory was being inserted incorrectly, fixed. - Boot-time driver objects were being inserted for no reason, call removed. - Support the only case of ObInsertObject where it is OK to call it without an output handle. This codepath will only charge quota instead of creating the full-blown handle.

ion at svn.reactos.org ion at svn.reactos.org
Tue Jan 9 09:38:08 CET 2007


Author: ion
Date: Tue Jan  9 11:38:07 2007
New Revision: 25394

URL: http://svn.reactos.org/svn/reactos?rev=25394&view=rev
Log:
- Enable sanity check in ObInsertObject to catch assholes that were calling it incorrectly (without a Handle output parameter, which is only allowed in a specific scenario). Changes:
   - Registry code which was calling ObInsertObject for no reason at all. Now an ugly hack has been added to Cm code to perform the only operation the insert did -> to free the create info.
   - SeSubProcessToken was broken and calling it incorrectly, fixed.
   - \Device\PhysicalMemory was being inserted incorrectly, fixed.
   - Boot-time driver objects were being inserted for no reason, call removed.
- Support the only case of ObInsertObject where it is OK to call it without an output handle. This codepath will only charge quota instead of creating the full-blown handle.

Modified:
    trunk/reactos/ntoskrnl/cm/registry.c
    trunk/reactos/ntoskrnl/cm/regobj.c
    trunk/reactos/ntoskrnl/io/iomgr/driver.c
    trunk/reactos/ntoskrnl/mm/section.c
    trunk/reactos/ntoskrnl/ob/obhandle.c
    trunk/reactos/ntoskrnl/se/token.c

Modified: trunk/reactos/ntoskrnl/cm/registry.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/registry.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/registry.c (original)
+++ trunk/reactos/ntoskrnl/cm/registry.c Tue Jan  9 11:38:07 2007
@@ -597,6 +597,7 @@
 	RtlFreeUnicodeString(&RemainingPath);
 	return Status;
       }
+#if 0
     DPRINT("Inserting Key into Object Tree\n");
     Status =  ObInsertObject((PVOID)NewKey,
                              NULL,
@@ -605,6 +606,11 @@
                              NULL,
                              NULL);
   DPRINT("Status %x\n", Status);
+#else
+    /* Free the create information */
+    ObpFreeAndReleaseCapturedAttributes(OBJECT_TO_OBJECT_HEADER(NewKey)->ObjectCreateInfo);
+    OBJECT_TO_OBJECT_HEADER(NewKey)->ObjectCreateInfo = NULL;
+#endif
   NewKey->Flags = 0;
   NewKey->SubKeyCounts = 0;
   NewKey->SubKeys = NULL;

Modified: trunk/reactos/ntoskrnl/cm/regobj.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/regobj.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/regobj.c (original)
+++ trunk/reactos/ntoskrnl/cm/regobj.c Tue Jan  9 11:38:07 2007
@@ -382,6 +382,7 @@
           RtlFreeUnicodeString(&KeyName);
           return(Status);
         }
+#if 0
       DPRINT("Inserting Key into Object Tree\n");
       Status = ObInsertObject((PVOID)FoundObject,
                               NULL,
@@ -390,6 +391,11 @@
                               NULL,
                               NULL);
       DPRINT("Status %x\n", Status);
+#else
+/* Free the create information */
+ObpFreeAndReleaseCapturedAttributes(OBJECT_TO_OBJECT_HEADER(FoundObject)->ObjectCreateInfo);
+OBJECT_TO_OBJECT_HEADER(FoundObject)->ObjectCreateInfo = NULL;
+#endif
 
       /* Add the keep-alive reference */
       ObReferenceObject(FoundObject);

Modified: trunk/reactos/ntoskrnl/io/iomgr/driver.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/driver.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/driver.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/driver.c Tue Jan  9 11:38:07 2007
@@ -262,18 +262,6 @@
       else
          ExFreePool(Buffer);
    }
-
-
-   Status = ObInsertObject(Object,
-                           NULL,
-                           FILE_ALL_ACCESS,
-                           0,
-                           NULL,
-                           NULL);
-   if (!NT_SUCCESS(Status))
-   {
-      return Status;
-   }  
 
    *DriverObject = Object;
 

Modified: trunk/reactos/ntoskrnl/mm/section.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c (original)
+++ trunk/reactos/ntoskrnl/mm/section.c Tue Jan  9 11:38:07 2007
@@ -2216,6 +2216,7 @@
    OBJECT_ATTRIBUTES Obj;
    UNICODE_STRING Name = RTL_CONSTANT_STRING(L"\\Device\\PhysicalMemory");
    LARGE_INTEGER SectionSize;
+   HANDLE Handle;
 
    /*
     * Create the section mapping physical memory
@@ -2244,11 +2245,12 @@
                            SECTION_ALL_ACCESS,
                            0,
                            NULL,
-                           NULL);
+                           &Handle);
    if (!NT_SUCCESS(Status))
    {
       ObDereferenceObject(PhysSection);
    }
+   ObCloseHandle(Handle, KernelMode);
    PhysSection->AllocationAttributes |= SEC_PHYSICALMEMORY;
    PhysSection->Segment->Flags &= ~MM_PAGEFILE_SEGMENT;
 

Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Tue Jan  9 11:38:07 2007
@@ -2329,6 +2329,7 @@
     OB_OPEN_REASON OpenReason;
     KPROCESSOR_MODE PreviousMode;
     NTSTATUS Status = STATUS_SUCCESS, RealStatus;
+    BOOLEAN IsNewObject;
     PAGED_CODE();
 
     /* Get the Header */
@@ -2371,34 +2372,28 @@
         ObjectName = &ObjectNameInfo->Name;
     }
 
-    /* Sanity check, but broken on ROS due to Cm */
-#if 0
+    /* Sanity check */
     ASSERT((Handle) ||
            ((ObjectPointerBias == 0) &&
             (ObjectName == NULL) &&
             (ObjectType->TypeInfo.SecurityRequired) &&
             (NewObject == NULL)));
-#endif
 
     /* Check if the object is unnamed and also doesn't have security */
     PreviousMode = KeGetPreviousMode();
     if (!(ObjectType->TypeInfo.SecurityRequired) && !(ObjectName))
     {
-        /* ReactOS HACK */
-        if (Handle)
-        {
-            /* Assume failure */
-            *Handle = NULL;
-
-            /* Create the handle */
-            Status = ObpCreateUnnamedHandle(Object,
-                                            DesiredAccess,
-                                            ObjectPointerBias + 1,
-                                            ObjectCreateInfo->Attributes,
-                                            PreviousMode,
-                                            NewObject,
-                                            Handle);
-        }
+        /* Assume failure */
+        *Handle = NULL;
+
+        /* Create the handle */
+        Status = ObpCreateUnnamedHandle(Object,
+                                        DesiredAccess,
+                                        ObjectPointerBias + 1,
+                                        ObjectCreateInfo->Attributes,
+                                        PreviousMode,
+                                        NewObject,
+                                        Handle);
 
         /* Free the create information */
         ObpFreeAndReleaseCapturedAttributes(ObjectCreateInfo);
@@ -2408,7 +2403,7 @@
         if (ObjectNameInfo) ObpDecrementQueryReference(ObjectNameInfo);
 
         /* Remove the extra keep-alive reference */
-        if (Handle) ObDereferenceObject(Object);
+        ObDereferenceObject(Object);
 
         /* Return */
         OBTRACE(OB_HANDLE_DEBUG,
@@ -2590,12 +2585,7 @@
     /* Save the actual status until here */
     RealStatus = Status;
 
-    /* HACKHACK: Because of ROS's incorrect startup, this can be called
-     * without a valid Process until I finalize the startup patch,
-     * so don't create a handle if this is the case. We also don't create
-     * a handle if Handle is NULL when the Registry Code calls it, because
-     * the registry code totally bastardizes the Ob and needs to be fixed
-     */
+    /* Check if caller wants us to create a handle */
     ObjectHeader->ObjectCreateInfo = NULL;
     if (Handle)
     {
@@ -2610,28 +2600,38 @@
                                  PreviousMode,
                                  NewObject,
                                  Handle);
-    }
-
-    /* Check if creating the handle failed */
-    if (!NT_SUCCESS(Status))
-    {
-        /* If the object had a name, backout everything */
-        if (ObjectName) ObpDeleteNameCheck(Object);
-    }
-
-    /* Check our final status */
-    if (!NT_SUCCESS(Status))
-    {
-        /* Return the status of the failure */
-        *Handle = NULL;
-        RealStatus = Status;
-    }
-
-    /* Remove a query reference */
-    if (ObjectNameInfo) ObpDecrementQueryReference(ObjectNameInfo);
-
-    /* Remove the extra keep-alive reference */
-    if (Handle) ObDereferenceObject(Object);
+        if (!NT_SUCCESS(Status))
+        {
+            /* If the object had a name, backout everything */
+            if (ObjectName) ObpDeleteNameCheck(Object);
+
+            /* Return the status of the failure */
+            *Handle = NULL;
+            RealStatus = Status;
+        }
+
+        /* Remove a query reference */
+        if (ObjectNameInfo) ObpDecrementQueryReference(ObjectNameInfo);
+
+        /* Remove the extra keep-alive reference */
+        ObDereferenceObject(Object);
+    }
+    else
+    {
+        /* Otherwise, lock the object type */
+        ObpEnterObjectTypeMutex(ObjectType);
+
+        /* And charge quota for the process to make it appear as used */
+        RealStatus = ObpChargeQuotaForObject(ObjectHeader,
+                                             ObjectType,
+                                             &IsNewObject);
+
+        /* Release the lock */
+        ObpLeaveObjectTypeMutex(ObjectType);
+
+        /* Check if we failed and dereference the object if so */
+        if (!NT_SUCCESS(RealStatus)) ObDereferenceObject(Object);
+    }
 
     /* We can delete the Create Info now */
     ObpFreeAndReleaseCapturedAttributes(ObjectCreateInfo);

Modified: trunk/reactos/ntoskrnl/se/token.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/token.c?rev=25394&r1=25393&r2=25394&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/se/token.c (original)
+++ trunk/reactos/ntoskrnl/se/token.c Tue Jan  9 11:38:07 2007
@@ -350,7 +350,7 @@
         Status = ObInsertObject(NewToken,
                                 NULL,
                                 0,
-                                1,
+                                0,
                                 NULL,
                                 NULL);
         if (NT_SUCCESS(Status))
@@ -658,6 +658,8 @@
     RtlZeroMemory(&ObjectTypeInitializer, sizeof(ObjectTypeInitializer));
     RtlInitUnicodeString(&Name, L"Token");
     ObjectTypeInitializer.Length = sizeof(ObjectTypeInitializer);
+    ObjectTypeInitializer.InvalidAttributes = OBJ_OPENLINK;
+    ObjectTypeInitializer.SecurityRequired = TRUE;
     ObjectTypeInitializer.DefaultPagedPoolCharge = sizeof(TOKEN);
     ObjectTypeInitializer.GenericMapping = SepTokenMapping;
     ObjectTypeInitializer.PoolType = PagedPool;




More information about the Ros-diffs mailing list