[ros-diffs] [ion] 25400: - Fix Port and Section Object Type creation by specifying a valid ValidAccessMask when creating the types. - NTLPC "Branch": Ports need to maintain a Type List. - Use proper access mode in parse callbacks. - Properly validate the access mask given to ObpCreate(Unnamed)Handle and only grant valid bits according to ValidAccessMask. - Use InterlockedExchangeAdd for reference count bias instead of looping on a single increment. - Only return the object if the caller did any bias to it. - Detach from the process much later, since exclusive process support and handle table database needs to be in the same context as the owner. - Add audit calls to ObpCreateHandle. - Add stubbed out calls to ObpCleanupDirectoryLookup in ObpCreateHandle.
ion at svn.reactos.org
ion at svn.reactos.org
Tue Jan 9 18:18:23 CET 2007
Author: ion
Date: Tue Jan 9 20:18:22 2007
New Revision: 25400
URL: http://svn.reactos.org/svn/reactos?rev=25400&view=rev
Log:
- Fix Port and Section Object Type creation by specifying a valid ValidAccessMask when creating the types.
- NTLPC "Branch": Ports need to maintain a Type List.
- Use proper access mode in parse callbacks.
- Properly validate the access mask given to ObpCreate(Unnamed)Handle and only grant valid bits according to ValidAccessMask.
- Use InterlockedExchangeAdd for reference count bias instead of looping on a single increment.
- Only return the object if the caller did any bias to it.
- Detach from the process much later, since exclusive process support and handle table database needs to be in the same context as the owner.
- Add audit calls to ObpCreateHandle.
- Add stubbed out calls to ObpCleanupDirectoryLookup in ObpCreateHandle.
Modified:
trunk/reactos/ntoskrnl/KrnlFun.c
trunk/reactos/ntoskrnl/lpc/ntlpc/port.c
trunk/reactos/ntoskrnl/lpc/port.c
trunk/reactos/ntoskrnl/mm/section.c
trunk/reactos/ntoskrnl/ob/obhandle.c
trunk/reactos/ntoskrnl/ob/obname.c
Modified: trunk/reactos/ntoskrnl/KrnlFun.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/KrnlFun.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/KrnlFun.c (original)
+++ trunk/reactos/ntoskrnl/KrnlFun.c Tue Jan 9 20:18:22 2007
@@ -11,8 +11,7 @@
//
// Ob:
// - Add Directory Lock.
-// - Strengthen code with debug checks and assertions.
-// - Fix FIXMEs/commented out code.
+// - Add Object Table Referencing.
//
// Ex:
// - Fixup existing code that talks to Ke.
Modified: trunk/reactos/ntoskrnl/lpc/ntlpc/port.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/lpc/ntlpc/port.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/lpc/ntlpc/port.c (original)
+++ trunk/reactos/ntoskrnl/lpc/ntlpc/port.c Tue Jan 9 20:18:22 2007
@@ -55,6 +55,7 @@
ObjectTypeInitializer.CloseProcedure = LpcpClosePort;
ObjectTypeInitializer.DeleteProcedure = LpcpDeletePort;
ObjectTypeInitializer.ValidAccessMask = PORT_ALL_ACCESS;
+ ObjectTypeInitializer.MaintainTypeList = TRUE;
ObCreateObjectType(&Name,
&ObjectTypeInitializer,
NULL,
Modified: trunk/reactos/ntoskrnl/lpc/port.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/lpc/port.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/lpc/port.c (original)
+++ trunk/reactos/ntoskrnl/lpc/port.c Tue Jan 9 20:18:22 2007
@@ -56,6 +56,7 @@
ObjectTypeInitializer.UseDefaultObject = TRUE;
ObjectTypeInitializer.CloseProcedure = LpcpClosePort;
ObjectTypeInitializer.DeleteProcedure = LpcpDeletePort;
+ ObjectTypeInitializer.ValidAccessMask = PORT_ALL_ACCESS;
ObCreateObjectType(&Name, &ObjectTypeInitializer, NULL, &LpcPortObjectType);
LpcpNextMessageId = 0;
Modified: trunk/reactos/ntoskrnl/mm/section.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c (original)
+++ trunk/reactos/ntoskrnl/mm/section.c Tue Jan 9 20:18:22 2007
@@ -2277,6 +2277,7 @@
ObjectTypeInitializer.GenericMapping = MmpSectionMapping;
ObjectTypeInitializer.DeleteProcedure = MmpDeleteSection;
ObjectTypeInitializer.CloseProcedure = MmpCloseSection;
+ ObjectTypeInitializer.ValidAccessMask = SECTION_ALL_ACCESS;
ObCreateObjectType(&Name, &ObjectTypeInitializer, NULL, &MmSectionObjectType);
return(STATUS_SUCCESS);
Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Tue Jan 9 20:18:22 2007
@@ -1031,14 +1031,16 @@
POBJECT_HEADER ObjectHeader;
HANDLE Handle;
KAPC_STATE ApcState;
- BOOLEAN AttachedToProcess = FALSE;
+ BOOLEAN AttachedToProcess = FALSE, KernelHandle = FALSE;
PVOID HandleTable;
NTSTATUS Status;
- ULONG i;
+ ACCESS_MASK GrantedAccess;
+ POBJECT_TYPE ObjectType;
PAGED_CODE();
/* Get the object header and type */
ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object);
+ ObjectType = ObjectHeader->Type;
OBTRACE(OB_HANDLE_DEBUG,
"%s - Creating handle for: %p. UNNAMED. HC LC %lx %lx\n",
__FUNCTION__,
@@ -1051,6 +1053,7 @@
{
/* Set the handle table */
HandleTable = ObpKernelHandleTable;
+ KernelHandle = TRUE;
/* Check if we're not in the system process */
if (PsGetCurrentProcess() != PsInitialSystemProcess)
@@ -1082,8 +1085,7 @@
return Status;
}
- /* Save the object header (assert its validity too) */
- ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED);
+ /* Save the object header */
NewEntry.Object = ObjectHeader;
/* Mask out the internal attributes */
@@ -1092,20 +1094,20 @@
EX_HANDLE_ENTRY_INHERITABLE |
EX_HANDLE_ENTRY_AUDITONCLOSE);
- /* Save the access mask */
- NewEntry.GrantedAccess = DesiredAccess;
+ /* Remove what's not in the valid access mask */
+ GrantedAccess = DesiredAccess & (ObjectType->TypeInfo.ValidAccessMask |
+ ACCESS_SYSTEM_SECURITY);
/* Handle extra references */
if (AdditionalReferences)
{
- /* Make a copy in case we fail later below */
- i = AdditionalReferences;
- while (i--)
- {
- /* Increment the count */
- InterlockedIncrement(&ObjectHeader->PointerCount);
- }
- }
+ /* Add them to the header */
+ InterlockedExchangeAdd(&ObjectHeader->PointerCount,
+ AdditionalReferences);
+ }
+
+ /* Save the access mask */
+ NewEntry.GrantedAccess = GrantedAccess;
/*
* Create the actual handle. We'll need to do this *after* calling
@@ -1118,22 +1120,26 @@
NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess);
Handle = ExCreateHandle(HandleTable, &NewEntry);
- /* Detach if needed */
- if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
-
/* Make sure we got a handle */
if (Handle)
{
/* Check if this was a kernel handle */
- if (HandleAttributes & OBJ_KERNEL_HANDLE)
- {
- /* Set the kernel handle bit */
- Handle = ObMarkHandleAsKernelHandle(Handle);
- }
+ if (KernelHandle) Handle = ObMarkHandleAsKernelHandle(Handle);
/* Return handle and object */
*ReturnedHandle = Handle;
- if (ReturnedObject) *ReturnedObject = Object;
+
+ /* Return the new object only if caller wanted it biased */
+ if ((AdditionalReferences) && (ReturnedObject))
+ {
+ /* Return it */
+ *ReturnedObject = Object;
+ }
+
+ /* Detach if needed */
+ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
+
+ /* Trace and return */
OBTRACE(OB_HANDLE_DEBUG,
"%s - Returning Handle: %lx HC LC %lx %lx\n",
__FUNCTION__,
@@ -1144,16 +1150,25 @@
}
/* Handle extra references */
- while (AdditionalReferences--)
- {
- /* Decrement the count */
- InterlockedDecrement(&ObjectHeader->PointerCount);
+ if (AdditionalReferences == 1)
+ {
+ /* Dereference the object once */
+ ObDereferenceObject(Object);
+ }
+ else if (AdditionalReferences > 1)
+ {
+ /* Dereference it many times */
+ InterlockedExchangeAdd(&ObjectHeader->PointerCount,
+ -AdditionalReferences);
}
/* Decrement the handle count and detach */
ObpDecrementHandleCount(&ObjectHeader->Body,
PsGetCurrentProcess(),
- NewEntry.GrantedAccess);
+ GrantedAccess);
+
+ /* Detach and fail */
+ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
return STATUS_INSUFFICIENT_RESOURCES;
}
@@ -1191,7 +1206,7 @@
*
* @return <FILLMEIN>.
*
-* @remarks Cleans up the Lookup Context on success.
+* @remarks Cleans up the Lookup Context on return.
*
*--*/
NTSTATUS
@@ -1211,11 +1226,12 @@
POBJECT_HEADER ObjectHeader;
HANDLE Handle;
KAPC_STATE ApcState;
- BOOLEAN AttachedToProcess = FALSE;
+ BOOLEAN AttachedToProcess = FALSE, KernelHandle = FALSE;
POBJECT_TYPE ObjectType;
PVOID HandleTable;
NTSTATUS Status;
- ULONG i;
+ ACCESS_MASK DesiredAccess, GrantedAccess;
+ PAUX_DATA AuxData;
PAGED_CODE();
/* Get the object header and type */
@@ -1230,13 +1246,19 @@
ObjectHeader->PointerCount);
/* Check if the types match */
- if ((Type) && (ObjectType != Type)) return STATUS_OBJECT_TYPE_MISMATCH;
+ if ((Type) && (ObjectType != Type))
+ {
+ /* They don't, cleanup */
+ //if (Context) ObpCleanupDirectoryLookup(Object, Context);
+ return STATUS_OBJECT_TYPE_MISMATCH;
+ }
/* Check if this is a kernel handle */
if ((HandleAttributes & OBJ_KERNEL_HANDLE) && (AccessMode == KernelMode))
{
/* Set the handle table */
HandleTable = ObpKernelHandleTable;
+ KernelHandle = TRUE;
/* Check if we're not in the system process */
if (PsGetCurrentProcess() != PsInitialSystemProcess)
@@ -1265,13 +1287,17 @@
* We failed (meaning security failure, according to NT Internals)
* detach and return
*/
+ //if (Context) ObpCleanupDirectoryLookup(Object, Context);
if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
return Status;
}
- /* Save the object header (assert its validity too) */
- ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED);
- NewEntry.Object = ObjectHeader;
+ /* Check if we are doing audits on close */
+ if (AccessState->GenerateOnClose)
+ {
+ /* Force the attribute on */
+ HandleAttributes|= EX_HANDLE_ENTRY_AUDITONCLOSE;
+ }
/* Mask out the internal attributes */
NewEntry.ObAttributes |= HandleAttributes &
@@ -1279,21 +1305,35 @@
EX_HANDLE_ENTRY_INHERITABLE |
EX_HANDLE_ENTRY_AUDITONCLOSE);
- /* Save the access mask */
- NewEntry.GrantedAccess = AccessState->RemainingDesiredAccess |
- AccessState->PreviouslyGrantedAccess;
+ /* Get the original desired access */
+ DesiredAccess = AccessState->RemainingDesiredAccess |
+ AccessState->PreviouslyGrantedAccess;
+
+ /* Remove what's not in the valid access mask */
+ GrantedAccess = DesiredAccess & (ObjectType->TypeInfo.ValidAccessMask |
+ ACCESS_SYSTEM_SECURITY);
+
+ /* Update the value in the access state */
+ AccessState->PreviouslyGrantedAccess = GrantedAccess;
+
+ /* Get the auxiliary data */
+ AuxData = AccessState->AuxData;
/* Handle extra references */
if (AdditionalReferences)
{
- /* Make a copy in case we fail later below */
- i = AdditionalReferences;
- while (i--)
- {
- /* Increment the count */
- InterlockedIncrement(&ObjectHeader->PointerCount);
- }
- }
+ /* Add them to the header */
+ InterlockedExchangeAdd(&ObjectHeader->PointerCount, AdditionalReferences);
+ }
+
+ /* Now we can release the object */
+ //if (Context) ObpCleanupDirectoryLookup(Object, Context);
+
+ /* Save the object header */
+ NewEntry.Object = ObjectHeader;
+
+ /* Save the access mask */
+ NewEntry.GrantedAccess = GrantedAccess;
/*
* Create the actual handle. We'll need to do this *after* calling
@@ -1306,22 +1346,59 @@
NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess);
Handle = ExCreateHandle(HandleTable, &NewEntry);
- /* Detach if needed */
- if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
-
/* Make sure we got a handle */
if (Handle)
{
+ /* Check if we have auxiliary data */
+ if (AuxData)
+ {
+ /* FIXME: Change handle security */
+ }
+
/* Check if this was a kernel handle */
- if (HandleAttributes & OBJ_KERNEL_HANDLE)
- {
- /* Set the kernel handle bit */
- Handle = ObMarkHandleAsKernelHandle(Handle);
- }
-
- /* Return handle and object */
+ if (KernelHandle) Handle = ObMarkHandleAsKernelHandle(Handle);
+
+ /* Return it */
*ReturnedHandle = Handle;
- if (ReturnedObject) *ReturnedObject = Object;
+
+ /* Check if we need to generate on audit */
+ if (AccessState->GenerateAudit)
+ {
+ /* Audit the handle creation */
+ //SeAuditHandleCreation(AccessState, Handle);
+ }
+
+ /* Check if this was a create */
+ if (OpenReason == ObCreateHandle)
+ {
+ /* Check if we need to audit the privileges */
+ if ((AuxData->PrivilegeSet) &&
+ (AuxData->PrivilegeSet->PrivilegeCount))
+ {
+ /* Do the audit */
+#if 0
+ SePrivilegeObjectAuditAlarm(Handle,
+ &AccessState->
+ SubjectSecurityContext,
+ GrantedAccess,
+ AuxData->PrivilegeSet,
+ TRUE,
+ ExGetPreviousMode());
+#endif
+ }
+ }
+
+ /* Return the new object only if caller wanted it biased */
+ if ((AdditionalReferences) && (ReturnedObject))
+ {
+ /* Return it */
+ *ReturnedObject = Object;
+ }
+
+ /* Detach if needed */
+ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
+
+ /* Trace and return */
OBTRACE(OB_HANDLE_DEBUG,
"%s - Returning Handle: %lx HC LC %lx %lx\n",
__FUNCTION__,
@@ -1331,17 +1408,26 @@
return STATUS_SUCCESS;
}
- /* Handle extra references */
- while (AdditionalReferences--)
- {
- /* Increment the count */
- InterlockedDecrement(&ObjectHeader->PointerCount);
- }
-
/* Decrement the handle count and detach */
ObpDecrementHandleCount(&ObjectHeader->Body,
PsGetCurrentProcess(),
- NewEntry.GrantedAccess);
+ GrantedAccess);
+
+ /* Handle extra references */
+ if (AdditionalReferences == 1)
+ {
+ /* Dereference the object once */
+ ObDereferenceObject(Object);
+ }
+ else if (AdditionalReferences > 1)
+ {
+ /* Dereference it many times */
+ InterlockedExchangeAdd(&ObjectHeader->PointerCount,
+ -AdditionalReferences);
+ }
+
+ /* Detach if necessary and fail */
+ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
return STATUS_INSUFFICIENT_RESOURCES;
}
Modified: trunk/reactos/ntoskrnl/ob/obname.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obname.c?rev=25400&r1=25399&r2=25400&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obname.c (original)
+++ trunk/reactos/ntoskrnl/ob/obname.c Tue Jan 9 20:18:22 2007
@@ -292,19 +292,19 @@
IN POBP_LOOKUP_CONTEXT LookupContext,
OUT PVOID *FoundObject)
{
- PVOID RootDirectory;
- PVOID Directory = NULL, ParentDirectory = NULL;
PVOID Object;
POBJECT_HEADER ObjectHeader;
+ UNICODE_STRING ComponentName, RemainingName;
+ BOOLEAN InsideRoot = FALSE;
+ PDEVICE_MAP DeviceMap = NULL;
+ POBJECT_DIRECTORY Directory = NULL, ParentDirectory = NULL, RootDirectory;
+ POBJECT_DIRECTORY ReferencedDirectory = NULL, ReferencedParentDirectory = NULL;
+ KIRQL CalloutIrql;
+ OB_PARSE_METHOD ParseRoutine;
NTSTATUS Status;
+ KPROCESSOR_MODE AccessCheckMode;
PWCHAR NewName;
POBJECT_HEADER_NAME_INFO ObjectNameInfo;
- UNICODE_STRING RemainingName, ComponentName;
- BOOLEAN InsideRoot = FALSE;
- KPROCESSOR_MODE AccessCheckMode;
- OB_PARSE_METHOD ParseRoutine;
- KIRQL CalloutIrql;
- POBJECT_DIRECTORY ReferencedDirectory = NULL, ReferencedParentDirectory = NULL;
PAGED_CODE();
OBTRACE(OB_NAMESPACE_DEBUG,
"%s - Finding Object: %wZ. Expecting: %p\n",
@@ -337,7 +337,7 @@
0,
NULL,
AccessMode,
- &RootDirectory,
+ (PVOID*)&RootDirectory,
NULL);
if (!NT_SUCCESS(Status)) return Status;
@@ -377,7 +377,7 @@
Status = ParseRoutine(RootDirectory,
ObjectType,
AccessState,
- AccessMode,
+ AccessCheckMode,
Attributes,
ObjectName,
&RemainingName,
@@ -725,7 +725,7 @@
Status = ParseRoutine(Object,
ObjectType,
AccessState,
- AccessMode,
+ AccessCheckMode,
Attributes,
ObjectName,
&RemainingName,
@@ -873,7 +873,7 @@
}
/* Check if we have a device map and dereference it if so */
- //if (DeviceMap) ObfDereferenceDeviceMap(DeviceMap);
+ if (DeviceMap) ObfDereferenceDeviceMap(DeviceMap);
/* Check if we have a referenced directory and dereference it if so */
if (ReferencedDirectory) ObDereferenceObject(ReferencedDirectory);
More information about the Ros-diffs
mailing list