- Fix ObGetObjectSecurity to use the object type's pool type instead of assuming PagedPool.

- Re-wrote the way ObInsertObject handles security as described in Chapters 6 and 9 of Gl00my (ie: made it use ObGetObjectSecurity and ObAssignObjectSecurity; ironically, these functions already existed/are exported and could've been used since the start instead of duplicating code).
- Fix ObpReferenceCachedSecurityDescriptor only to touch the cached entry if it actually gets a non-NULL descriptor. Also improved it to return the referenced SD, instead of requiring the caller to do it manually.

svn path=/trunk/; revision=22279

reactos/ntoskrnl/include/internal/ob.h

@@ -172,9 +172,11 @@ NTSTATUS
 NTAPI
 ObpRemoveSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor);
 
-VOID
+PSECURITY_DESCRIPTOR
 NTAPI
-ObpReferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor);
+ObpReferenceCachedSecurityDescriptor(
+    IN PSECURITY_DESCRIPTOR SecurityDescriptor
+);
 
 VOID
 NTAPI

reactos/ntoskrnl/ob/obhandle.c

@@ -1167,8 +1167,8 @@ ObInsertObject(IN PVOID Object,
     PVOID FoundObject = NULL;
     POBJECT_HEADER FoundHeader = NULL;
     NTSTATUS Status = STATUS_SUCCESS;
-    PSECURITY_DESCRIPTOR NewSecurityDescriptor = NULL;
+    PSECURITY_DESCRIPTOR DirectorySd = NULL;
-    SECURITY_SUBJECT_CONTEXT SubjectContext;
+    BOOLEAN SdAllocated;
     OBP_LOOKUP_CONTEXT Context;
     POBJECT_HEADER_NAME_INFO ObjectNameInfo;
     ACCESS_STATE AccessState;
@@ -1252,48 +1252,58 @@ ObInsertObject(IN PVOID Object,
         }
     }
 
-    DPRINT("Security Assignment in progress\n");
+    /* Check if it's named or forces security */
-    SeCaptureSubjectContext(&SubjectContext);
+    if ((IsNamed) || (ObjectType->TypeInfo.SecurityRequired))
-
-    /* Build the new security descriptor */
-    Status = SeAssignSecurity((FoundHeader != NULL) ? FoundHeader->SecurityDescriptor : NULL,
-        (ObjectCreateInfo != NULL) ? ObjectCreateInfo->SecurityDescriptor : NULL,
-        &NewSecurityDescriptor,
-        (Header->Type == ObDirectoryType),
-        &SubjectContext,
-        &Header->Type->TypeInfo.GenericMapping,
-        PagedPool);
-
-    if (NT_SUCCESS(Status))
     {
-        DPRINT("NewSecurityDescriptor %p\n", NewSecurityDescriptor);
+        /* Make sure it's inserted into an object directory */
-
+        if ((ObjectNameInfo) && (ObjectNameInfo->Directory))
-        if (Header->Type->TypeInfo.SecurityProcedure != NULL)
         {
-            /* Call the security method */
+            /* Get the current descriptor */
-            Status = Header->Type->TypeInfo.SecurityProcedure(&Header->Body,
+            ObGetObjectSecurity(ObjectNameInfo->Directory,
-                AssignSecurityDescriptor,
+                                &DirectorySd,
-                0,
+                                &SdAllocated);
-                NewSecurityDescriptor,
-                NULL,
-                NULL,
-                NonPagedPool,
-                NULL);
-        }
-        else
-        {
-            /* Assign the security descriptor to the object header */
-            Status = ObpAddSecurityDescriptor(NewSecurityDescriptor,
-                &Header->SecurityDescriptor);
-            DPRINT("Object security descriptor %p\n", Header->SecurityDescriptor);
         }
 
-        /* Release the new security descriptor */
+        /* Now assign it */
-        SeDeassignSecurity(&NewSecurityDescriptor);
+        Status = ObAssignSecurity(PassedAccessState,
+                                  DirectorySd,
+                                  Object,
+                                  ObjectType);
+
+        /* Check if we captured one */
+        if (DirectorySd)
+        {
+            /* We did, release it */
+            DPRINT1("Here\n");
+            ObReleaseObjectSecurity(DirectorySd, SdAllocated);
+        }
+        else if (NT_SUCCESS(Status))
+        {
+            /* Other we didn't, but we were able to use the current SD */
+            SeReleaseSecurityDescriptor(ObjectCreateInfo->SecurityDescriptor,
+                                        ObjectCreateInfo->ProbeMode,
+                                        TRUE);
+
+            /* Clear the current one */
+            PassedAccessState->SecurityDescriptor =
+                ObjectCreateInfo->SecurityDescriptor = NULL;
+        }
     }
 
-    DPRINT("Security Complete\n");
+    /* Check if anything until now failed */
-    SeReleaseSubjectContext(&SubjectContext);
+    if (!NT_SUCCESS(Status))
+    {
+        /* We failed, dereference the object and delete the access state */
+        ObDereferenceObject(Object);
+        if (PassedAccessState == &AccessState)
+        {
+            /* We used a local one; delete it */
+            SeDeleteAccessState(PassedAccessState);
+        }
+
+        /* Return failure code */
+        return Status;
+    }
 
     /* HACKHACK: Because of ROS's incorrect startup, this can be called
     * without a valid Process until I finalize the startup patch,
@@ -1301,8 +1311,7 @@ ObInsertObject(IN PVOID Object,
     * 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
     */
-    DPRINT("Creating handle\n");
+    if (Handle)
-    if (Handle != NULL)
     {
         /* Create the handle */
         Status = ObpCreateHandle(OpenReason,
@@ -1338,7 +1347,6 @@ ObInsertObject(IN PVOID Object,
     }
 
     /* Return failure code */
-    DPRINT("Status %x\n", Status);
     return Status;
 }
 

reactos/ntoskrnl/ob/sdcache.c

@@ -306,28 +306,32 @@ ObpRemoveSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)
   return STATUS_SUCCESS;
 }
 
-
+PSECURITY_DESCRIPTOR
-VOID
 NTAPI
 ObpReferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)
 {
     PSD_CACHE_ENTRY CacheEntry;
 
-  DPRINT("ObpReferenceCachedSecurityDescriptor() called\n");
+    /* Lock the cache */
-
     ObpSdCacheLock();
 
-  CacheEntry = (PSD_CACHE_ENTRY)((ULONG_PTR)SecurityDescriptor - sizeof(SD_CACHE_ENTRY));
+    /* Make sure we got a descriptor */
+    if (SecurityDescriptor)
+    {
+        /* Get the entry */
+        CacheEntry = (PSD_CACHE_ENTRY)((ULONG_PTR)SecurityDescriptor -
+                                       sizeof(SD_CACHE_ENTRY));
 
+        /* Reference it */
         CacheEntry->RefCount++;
         DPRINT("RefCount %lu\n", CacheEntry->RefCount);
+    }
 
+    /* Unlock the cache and return the descriptor */
     ObpSdCacheUnlock();
-
+    return SecurityDescriptor;
-  DPRINT("ObpReferenceCachedSecurityDescriptor() done\n");
 }
 
-
 VOID
 NTAPI
 ObpDereferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)

reactos/ntoskrnl/ob/security.c

@@ -117,9 +117,9 @@ ObGetObjectSecurity(IN PVOID Object,
     /* Check if the object uses default security */
     if (Type->TypeInfo.SecurityProcedure == SeDefaultObjectMethod)
     {
-        /* Reference the descriptor and return it */
+        /* Reference the descriptor */
+        *SecurityDescriptor =
             ObpReferenceCachedSecurityDescriptor(Header->SecurityDescriptor);
-        *SecurityDescriptor = Header->SecurityDescriptor;
 
         /* Tell the caller that we didn't have to allocate anything */
         *MemoryAllocated = FALSE;
@@ -134,10 +134,10 @@ ObGetObjectSecurity(IN PVOID Object,
                                               GROUP_SECURITY_INFORMATION |
                                               DACL_SECURITY_INFORMATION |
                                               SACL_SECURITY_INFORMATION,
-                                              NULL,
+                                              *SecurityDescriptor,
                                               &Length,
                                               &Header->SecurityDescriptor,
-                                              PagedPool,
+                                              Type->TypeInfo.PoolType,
                                               &Type->TypeInfo.GenericMapping);
     if (Status != STATUS_BUFFER_TOO_SMALL) return Status;
 
@@ -158,7 +158,7 @@ ObGetObjectSecurity(IN PVOID Object,
                                               *SecurityDescriptor,
                                               &Length,
                                               &Header->SecurityDescriptor,
-                                              PagedPool,
+                                              Type->TypeInfo.PoolType,
                                               &Type->TypeInfo.GenericMapping);
     if (!NT_SUCCESS(Status))
     {