Author: ion
Date: Thu Jun 8 10:17:46 2006
New Revision: 22280
URL: http://svn.reactos.ru/svn/reactos?rev=22280&view=rev
Log:
- ObFindObject should return the actual object being inserted, in the insert case (otherwise it always returns the root-directory instead...)
- Add check for optimized case for objects with no name and no security, but not implemented (ObpIncrementUnnamedHandleCount in Gl00my docs), since I need a better translation then babelfish's.
- Fix ObInsertObject to save the Security Descriptor in the Access State structure. Gl00my mentions this isn't absorbed by SeCreateAccessCheck and I just noticed that too.
- We only need to perform security checks for a new object, in ObInsertObject, not if the object already existed.
- Added proper backout+failure code in ObInsertObject if lookup failed, and also look out for mismatch/exists/collision cases (implemented using simple logic).
Modified:
trunk/reactos/ntoskrnl/ob/obhandle.c
trunk/reactos/ntoskrnl/ob/obname.c
Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Thu Jun 8 10:17:46 2006
@@ -1175,6 +1175,7 @@
AUX_DATA AuxData;
BOOLEAN IsNamed = FALSE;
OB_OPEN_REASON OpenReason = ObCreateHandle;
+ static int LostOptimizations = 0;
PAGED_CODE();
/* Get the Header and Create Info */
@@ -1182,6 +1183,22 @@
ObjectCreateInfo = Header->ObjectCreateInfo;
ObjectNameInfo = OBJECT_HEADER_TO_NAME_INFO(Header);
ObjectType = Header->Type;
+
+ /* Check if this is an named object */
+ if ((ObjectNameInfo) && (ObjectNameInfo->Name.Buffer)) IsNamed = TRUE;
+
+ /* Check if the object is unnamed and also doesn't have security */
+ if ((!ObjectType->TypeInfo.SecurityRequired) && !(IsNamed))
+ {
+ /*
+ * FIXME: TODO (Optimized path through ObpIncrement*UnNamed*HandleCount).
+ * Described in chapter 6 of Gl00my, but babelfish translation isn't fully
+ * clear, so waiting on Aleksey's translation. Currently just profiling.
+ * (about ~500 calls per boot - not critical atm).
+ */
+ ++LostOptimizations;
+ DPRINT("Optimized case could've be taken: %d times!\n", LostOptimizations);
+ }
/* Check if we didn't get an access state */
if (!PassedAccessState)
@@ -1200,8 +1217,9 @@
}
}
- /* Check if this is an named object */
- if ((ObjectNameInfo) && (ObjectNameInfo->Name.Buffer)) IsNamed = TRUE;
+ /* Save the security descriptor */
+ PassedAccessState->SecurityDescriptor =
+ ObjectCreateInfo->SecurityDescriptor;
/* Check if the object is named */
if (IsNamed)
@@ -1218,12 +1236,48 @@
ObjectCreateInfo->SecurityQos,
ObjectCreateInfo->ParseContext,
Object);
- if (!NT_SUCCESS(Status)) return Status;
-
- if (FoundObject)
- {
- DPRINT("Getting header: %x\n", FoundObject);
+ /* Check if we found an object that doesn't match the one requested */
+ if ((NT_SUCCESS(Status)) && (FoundObject) && (Object != FoundObject))
+ {
+ /* This means we're opening an object, not creating a new one */
FoundHeader = OBJECT_TO_OBJECT_HEADER(FoundObject);
+ OpenReason = ObOpenHandle;
+
+ /* Make sure the caller said it's OK to do this */
+ if (ObjectCreateInfo->Attributes & OBJ_OPENIF)
+ {
+ /* He did, but did he want this type? */
+ if (ObjectType != FoundHeader->Type)
+ {
+ /* Wrong type, so fail */
+ Status = STATUS_OBJECT_TYPE_MISMATCH;
+ }
+ else
+ {
+ /* Right type, so warn */
+ Status = STATUS_OBJECT_NAME_EXISTS;
+ }
+ }
+ else
+ {
+ /* Caller wanted to create a new object, fail */
+ Status = STATUS_OBJECT_NAME_COLLISION;
+ }
+ }
+
+ /* Check if anything until now failed */
+ 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;
}
}
else
@@ -1252,57 +1306,60 @@
}
}
- /* Check if it's named or forces security */
- if ((IsNamed) || (ObjectType->TypeInfo.SecurityRequired))
- {
- /* Make sure it's inserted into an object directory */
- if ((ObjectNameInfo) && (ObjectNameInfo->Directory))
- {
- /* Get the current descriptor */
- ObGetObjectSecurity(ObjectNameInfo->Directory,
- &DirectorySd,
- &SdAllocated);
- }
-
- /* Now assign it */
- 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;
- }
- }
-
- /* Check if anything until now failed */
- 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;
+ /* Now check if this object is being created */
+ if (FoundObject == Object)
+ {
+ /* Check if it's named or forces security */
+ if ((IsNamed) || (ObjectType->TypeInfo.SecurityRequired))
+ {
+ /* Make sure it's inserted into an object directory */
+ if ((ObjectNameInfo) && (ObjectNameInfo->Directory))
+ {
+ /* Get the current descriptor */
+ ObGetObjectSecurity(ObjectNameInfo->Directory,
+ &DirectorySd,
+ &SdAllocated);
+ }
+
+ /* Now assign it */
+ Status = ObAssignSecurity(PassedAccessState,
+ DirectorySd,
+ Object,
+ ObjectType);
+
+ /* Check if we captured one */
+ if (DirectorySd)
+ {
+ /* We did, release it */
+ 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;
+ }
+ }
+
+ /* Check if anything until now failed */
+ 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
Modified: trunk/reactos/ntoskrnl/ob/obname.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obname.c?rev=22…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obname.c (original)
+++ trunk/reactos/ntoskrnl/ob/obname.c Thu Jun 8 10:17:46 2006
@@ -310,6 +310,7 @@
}
}
RtlFreeUnicodeString(RemainingPath);
+ *ReturnedObject = Insert;
}
else
{
Author: ion
Date: Thu Jun 8 06:56:59 2006
New Revision: 22278
URL: http://svn.reactos.ru/svn/reactos?rev=22278&view=rev
Log:
- I just noticed that ObInsertObject never got updated to deal with the improvements in ObFindObject and ACCESS_STATE usage, so made the following fixes:
* Create the ACCESS_STATE structure much earlier.
* Actually send the access state and parse context to ObFindObject, when called from ObInsertObject (this should fix some hidden regressions, since they finally get an access state with access masks now).
* Remove some deprecated hacks.
* If inserting the handle failed, cleanup the name and remove it from the directory entry.
* Fix a memory leak because we weren't deleting the access state.
Modified:
trunk/reactos/ntoskrnl/ob/obhandle.c
Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Thu Jun 8 06:56:59 2006
@@ -1152,8 +1152,8 @@
/*
* @implemented
*/
-NTSTATUS
-STDCALL
+NTSTATUS
+NTAPI
ObInsertObject(IN PVOID Object,
IN PACCESS_STATE PassedAccessState OPTIONAL,
IN ACCESS_MASK DesiredAccess,
@@ -1163,6 +1163,7 @@
{
POBJECT_CREATE_INFORMATION ObjectCreateInfo;
POBJECT_HEADER Header;
+ POBJECT_TYPE ObjectType;
PVOID FoundObject = NULL;
POBJECT_HEADER FoundHeader = NULL;
NTSTATUS Status = STATUS_SUCCESS;
@@ -1172,27 +1173,51 @@
POBJECT_HEADER_NAME_INFO ObjectNameInfo;
ACCESS_STATE AccessState;
AUX_DATA AuxData;
+ BOOLEAN IsNamed = FALSE;
+ OB_OPEN_REASON OpenReason = ObCreateHandle;
PAGED_CODE();
/* Get the Header and Create Info */
Header = OBJECT_TO_OBJECT_HEADER(Object);
ObjectCreateInfo = Header->ObjectCreateInfo;
ObjectNameInfo = OBJECT_HEADER_TO_NAME_INFO(Header);
-
- /* First try to find the Object */
- if (ObjectNameInfo && ObjectNameInfo->Name.Buffer)
- {
+ ObjectType = Header->Type;
+
+ /* Check if we didn't get an access state */
+ if (!PassedAccessState)
+ {
+ /* Use our built-in access state */
+ PassedAccessState = &AccessState;
+ Status = SeCreateAccessState(&AccessState,
+ &AuxData,
+ DesiredAccess,
+ &ObjectType->TypeInfo.GenericMapping);
+ if (!NT_SUCCESS(Status))
+ {
+ /* Fail */
+ ObDereferenceObject(Object);
+ return Status;
+ }
+ }
+
+ /* Check if this is an named object */
+ if ((ObjectNameInfo) && (ObjectNameInfo->Name.Buffer)) IsNamed = TRUE;
+
+ /* Check if the object is named */
+ if (IsNamed)
+ {
+ /* Look it up */
Status = ObFindObject(ObjectCreateInfo->RootDirectory,
- &ObjectNameInfo->Name,
- ObjectCreateInfo->Attributes,
- KernelMode,
- &FoundObject,
- Header->Type,
- &Context,
- NULL,
- ObjectCreateInfo->SecurityQos,
- NULL,
- Object);
+ &ObjectNameInfo->Name,
+ ObjectCreateInfo->Attributes,
+ KernelMode,
+ &FoundObject,
+ ObjectType,
+ &Context,
+ PassedAccessState,
+ ObjectCreateInfo->SecurityQos,
+ ObjectCreateInfo->ParseContext,
+ Object);
if (!NT_SUCCESS(Status)) return Status;
if (FoundObject)
@@ -1211,8 +1236,7 @@
* called ObFindObject which already has this code.
* We basically kill 3-4 hacks and add 2 new ones.
*/
- if ((Header->Type == IoFileObjectType) ||
- (Header->Type->TypeInfo.OpenProcedure != NULL))
+ if (Header->Type == IoFileObjectType)
{
DPRINT("About to call Open Routine\n");
if (Header->Type == IoFileObjectType)
@@ -1225,22 +1249,6 @@
NULL);
DPRINT("Called IopCreateFile: %x\n", Status);
}
- else if (Header->Type->TypeInfo.OpenProcedure)
- {
- DPRINT("Calling %x\n", Header->Type->TypeInfo.OpenProcedure);
- Status = Header->Type->TypeInfo.OpenProcedure(ObCreateHandle,
- NULL,
- &Header->Body,
- 0,
- 0);
- }
-
- if (!NT_SUCCESS(Status))
- {
- DPRINT1("Create Failed\n");
- if (FoundObject) ObDereferenceObject(FoundObject);
- return Status;
- }
}
}
@@ -1287,24 +1295,6 @@
DPRINT("Security Complete\n");
SeReleaseSubjectContext(&SubjectContext);
- /* Check if we didn't get an access state */
- if (!PassedAccessState)
- {
- /* Use our built-in access state */
- PassedAccessState = &AccessState;
- Status = SeCreateAccessState(&AccessState,
- &AuxData,
- DesiredAccess,
- &Header->Type->TypeInfo.GenericMapping);
- if (!NT_SUCCESS(Status))
- {
- /* Fail */
- ObDereferenceObject(Object);
- return Status;
- }
- }
-
- /* Create the Handle */
/* 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
@@ -1314,7 +1304,8 @@
DPRINT("Creating handle\n");
if (Handle != NULL)
{
- Status = ObpCreateHandle(ObCreateHandle,
+ /* Create the handle */
+ Status = ObpCreateHandle(OpenReason,
&Header->Body,
NULL,
PassedAccessState,
@@ -1329,6 +1320,24 @@
Header->ObjectCreateInfo = NULL;
ObpFreeAndReleaseCapturedAttributes(ObjectCreateInfo);
+ /* Check if creating the handle failed */
+ if (!NT_SUCCESS(Status))
+ {
+ /* If the object had a name, backout everything */
+ if (IsNamed) ObpDeleteNameCheck(Object);
+ }
+
+ /* Remove the extra keep-alive reference */
+ //ObDereferenceObject(Object); FIXME: Will require massive changes
+
+ /* Check if we created our own access state */
+ if (PassedAccessState == &AccessState)
+ {
+ /* We used a local one; delete it */
+ SeDeleteAccessState(PassedAccessState);
+ }
+
+ /* Return failure code */
DPRINT("Status %x\n", Status);
return Status;
}
Author: ion
Date: Thu Jun 8 06:36:12 2006
New Revision: 22277
URL: http://svn.reactos.ru/svn/reactos?rev=22277&view=rev
Log:
- Fixup some comments and add Eric Kohl's name to this file, since he had worked on some of the original calls.
- Minor/trivial fixes to some Object Security APIs that were left in the dark:
* Use PagedPool instead of NonPagedPool memory, and also tag the allocation for debugging.
* Send needed data to the security procedure instead of NULL/0, including the Generic Mapping, and the actual Security Decriptor.
* Only un-assign the descriptor in case of failure, not all the time (the whole point of the API is to assign it!)
* Tell the caller that memory was NOT allocated if we failed to get the security descriptor.
Modified:
trunk/reactos/ntoskrnl/ob/security.c
Modified: trunk/reactos/ntoskrnl/ob/security.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/security.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/security.c (original)
+++ trunk/reactos/ntoskrnl/ob/security.c Thu Jun 8 06:36:12 2006
@@ -4,6 +4,7 @@
* FILE: ntoskrnl/ob/security.c
* PURPOSE: SRM Interface of the Object Manager
* PROGRAMMERS: Alex Ionescu (alex(a)relsoft.net)
+* Eric Kohl
*/
/* INCLUDES *****************************************************************/
@@ -65,12 +66,15 @@
NewDescriptor,
NULL,
NULL,
- NonPagedPool,
- NULL);
-
- /* Release the new security descriptor */
- SeDeassignSecurity(&NewDescriptor);
-
+ PagedPool,
+ &Type->TypeInfo.GenericMapping);
+ if (!NT_SUCCESS(Status))
+ {
+ /* Release the new security descriptor */
+ SeDeassignSecurity(&NewDescriptor);
+ }
+
+ /* Return to caller */
return Status;
}
@@ -101,61 +105,70 @@
OUT PBOOLEAN MemoryAllocated)
{
POBJECT_HEADER Header;
+ POBJECT_TYPE Type;
ULONG Length;
NTSTATUS Status;
PAGED_CODE();
+ /* Get the object header and type */
Header = OBJECT_TO_OBJECT_HEADER(Object);
- if (Header->Type == NULL) return STATUS_UNSUCCESSFUL;
-
- if (Header->Type->TypeInfo.SecurityProcedure == NULL)
- {
+ Type = Header->Type;
+
+ /* Check if the object uses default security */
+ if (Type->TypeInfo.SecurityProcedure == SeDefaultObjectMethod)
+ {
+ /* Reference the descriptor and return it */
ObpReferenceCachedSecurityDescriptor(Header->SecurityDescriptor);
*SecurityDescriptor = Header->SecurityDescriptor;
+
+ /* Tell the caller that we didn't have to allocate anything */
*MemoryAllocated = FALSE;
return STATUS_SUCCESS;
}
/* Get the security descriptor size */
Length = 0;
- Status = Header->Type->TypeInfo.SecurityProcedure(Object,
- QuerySecurityDescriptor,
- OWNER_SECURITY_INFORMATION |
- GROUP_SECURITY_INFORMATION |
- DACL_SECURITY_INFORMATION |
- SACL_SECURITY_INFORMATION,
- NULL,
- &Length,
- NULL,
- NonPagedPool,
- NULL);
+ Status = Type->TypeInfo.SecurityProcedure(Object,
+ QuerySecurityDescriptor,
+ OWNER_SECURITY_INFORMATION |
+ GROUP_SECURITY_INFORMATION |
+ DACL_SECURITY_INFORMATION |
+ SACL_SECURITY_INFORMATION,
+ NULL,
+ &Length,
+ &Header->SecurityDescriptor,
+ PagedPool,
+ &Type->TypeInfo.GenericMapping);
if (Status != STATUS_BUFFER_TOO_SMALL) return Status;
/* Allocate security descriptor */
- *SecurityDescriptor = ExAllocatePool(NonPagedPool, Length);
- if (*SecurityDescriptor == NULL) return STATUS_INSUFFICIENT_RESOURCES;
+ *SecurityDescriptor = ExAllocatePoolWithTag(PagedPool,
+ Length,
+ TAG('O', 'b', 'S', 'q'));
+ if (!(*SecurityDescriptor)) return STATUS_INSUFFICIENT_RESOURCES;
/* Query security descriptor */
- Status = Header->Type->TypeInfo.SecurityProcedure(Object,
- QuerySecurityDescriptor,
- OWNER_SECURITY_INFORMATION |
- GROUP_SECURITY_INFORMATION |
- DACL_SECURITY_INFORMATION |
- SACL_SECURITY_INFORMATION,
- *SecurityDescriptor,
- &Length,
- NULL,
- NonPagedPool,
- NULL);
+ *MemoryAllocated = TRUE;
+ Status = Type->TypeInfo.SecurityProcedure(Object,
+ QuerySecurityDescriptor,
+ OWNER_SECURITY_INFORMATION |
+ GROUP_SECURITY_INFORMATION |
+ DACL_SECURITY_INFORMATION |
+ SACL_SECURITY_INFORMATION,
+ *SecurityDescriptor,
+ &Length,
+ &Header->SecurityDescriptor,
+ PagedPool,
+ &Type->TypeInfo.GenericMapping);
if (!NT_SUCCESS(Status))
{
+ /* Free the descriptor and tell the caller we failed */
ExFreePool(*SecurityDescriptor);
- return Status;
- }
-
- *MemoryAllocated = TRUE;
-
- return STATUS_SUCCESS;
+ *MemoryAllocated = FALSE;
+ }
+
+ /* Return status */
+ return Status;
}
/*++
@@ -182,14 +195,18 @@
{
PAGED_CODE();
- if (SecurityDescriptor == NULL) return;
-
+ /* Nothing to do in this case */
+ if (!SecurityDescriptor) return;
+
+ /* Check if we had allocated it from memory */
if (MemoryAllocated)
{
+ /* Free it */
ExFreePool(SecurityDescriptor);
}
else
{
+ /* Otherwise this means we used an internal descriptor */
ObpDereferenceCachedSecurityDescriptor(SecurityDescriptor);
}
}