[ros-diffs] [ion] 22267: - Final fixes to ObpCreateHandle that I can think of before splitting Create/Increment (might be lacking some security checks or trace/database functionality, but I haven't found any info on the latter, while the former I will stub, but I don't have the skills to imlement (ObAssignObjectSecurity)): * Honour ObjectType passed to the function and fail if it doesn't match. * Use table-based logic instead of process-based logic for Kernel vs User-mode handles (same change that's been done all over the place, since it requires only one dereference of the process object). * Do the GENERIC/MAXIMUM_ALLOWED logic directly inside the ACCESS_STATE structure. * This is where we should call the OpenProcedure (acc. to Gl00my), but this kills win32k -- investigate, #ifed out for now. * Increase the object type's number of handles as well. * Set the handle table entry's ObAttributes correctly; the old code seems to have been messing that up. * Honour the AdditionalReferences parameter and do referencing bias if requested. * Honour the ReturnedObject parameter to return the object pointer back to the caller. * Add OBTRACEing to the function. * If we failed because a handle couldn't be allocated, use the distinguied STATUS_INSUFFICIENT_RESOURCES error code instead of the generic STATUS_UNSCUCESFFUL, and backout all the changes we made by calling ObpDecrementHandleCount.
ion at svn.reactos.org
ion at svn.reactos.org
Wed Jun 7 08:16:00 CEST 2006
Author: ion
Date: Wed Jun 7 10:15:59 2006
New Revision: 22267
URL: http://svn.reactos.ru/svn/reactos?rev=22267&view=rev
Log:
- Final fixes to ObpCreateHandle that I can think of before splitting Create/Increment (might be lacking some security checks or trace/database functionality, but I haven't found any info on the latter, while the former I will stub, but I don't have the skills to imlement (ObAssignObjectSecurity)):
* Honour ObjectType passed to the function and fail if it doesn't match.
* Use table-based logic instead of process-based logic for Kernel vs User-mode handles (same change that's been done all over the place, since it requires only one dereference of the process object).
* Do the GENERIC/MAXIMUM_ALLOWED logic directly inside the ACCESS_STATE structure.
* This is where we should call the OpenProcedure (acc. to Gl00my), but this kills win32k -- investigate, #ifed out for now.
* Increase the object type's number of handles as well.
* Set the handle table entry's ObAttributes correctly; the old code seems to have been messing that up.
* Honour the AdditionalReferences parameter and do referencing bias if requested.
* Honour the ReturnedObject parameter to return the object pointer back to the caller.
* Add OBTRACEing to the function.
* If we failed because a handle couldn't be allocated, use the distinguied STATUS_INSUFFICIENT_RESOURCES error code instead of the generic STATUS_UNSCUCESFFUL, and backout all the changes we made by calling ObpDecrementHandleCount.
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=22267&r1=22266&r2=22267&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Wed Jun 7 10:15:59 2006
@@ -359,7 +359,7 @@
// ObOpenHandle == 1, I'm guessing this is actually the
// OpenReason. Also makes sense since this function is shared
// by Duplication, Creation and Opening.
- IN PVOID ObjectBody,
+ IN PVOID Object,
IN POBJECT_TYPE Type OPTIONAL,
IN PACCESS_STATE AccessState,
IN ULONG AdditionalReferences,
@@ -369,92 +369,162 @@
OUT PHANDLE ReturnedHandle)
{
HANDLE_TABLE_ENTRY NewEntry;
- PEPROCESS Process, CurrentProcess;
+ PVOID HandleTable;
POBJECT_HEADER ObjectHeader;
+ POBJECT_TYPE ObjectType;
HANDLE Handle;
KAPC_STATE ApcState;
BOOLEAN AttachedToProcess = FALSE;
- ACCESS_MASK GrantedAccess;
-
- PAGED_CODE();
-
- DPRINT("ObpCreateHandle(obj %p)\n",ObjectBody);
-
- ASSERT(ObjectBody);
-
- CurrentProcess = PsGetCurrentProcess();
-
- ObjectHeader = OBJECT_TO_OBJECT_HEADER(ObjectBody);
-
- /* check that this is a valid kernel pointer */
+
+ /* Get the object header and type */
+ ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object);
+ ObjectType = ObjectHeader->Type;
+ OBTRACE("OBTRACE - %s - Creating handle for: %p. Reason: %lx. HC LC %lx %lx\n",
+ __FUNCTION__,
+ Object,
+ OpenReason,
+ ObjectHeader->HandleCount,
+ ObjectHeader->PointerCount);
+
+ /* Check if the types match */
+ if ((Type) && (ObjectType != Type))
+ {
+ /* They don't; fail */
+ DPRINT1("Type mismatch: %wZ, %wZ\n", &ObjectType->Name, &Type->Name);
+ 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;
+
+ /* Check if we're not in the system process */
+ if (PsGetCurrentProcess() != PsInitialSystemProcess)
+ {
+ /* Attach to the system process */
+ KeStackAttachProcess(&PsInitialSystemProcess->Pcb, &ApcState);
+ AttachedToProcess = TRUE;
+ }
+ }
+ else
+ {
+ /* Get the current handle table */
+ HandleTable = PsGetCurrentProcess()->ObjectTable;
+ }
+
+ /* Convert MAXIMUM_ALLOWED to GENERIC_ALL */
+ if (AccessState->RemainingDesiredAccess & MAXIMUM_ALLOWED)
+ {
+ /* Mask out MAXIMUM_ALLOWED and stick GENERIC_ALL instead */
+ AccessState->RemainingDesiredAccess &= ~MAXIMUM_ALLOWED;
+ AccessState->RemainingDesiredAccess |= GENERIC_ALL;
+ }
+
+ /* Check if we have to map the GENERIC mask */
+ if (AccessState->RemainingDesiredAccess & GENERIC_ACCESS)
+ {
+ /* Map it to the correct access masks */
+ RtlMapGenericMask(&AccessState->RemainingDesiredAccess,
+ &ObjectType->TypeInfo.GenericMapping);
+ }
+
+ /* Increase the handle count */
+ if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
+ {
+ /*
+ * FIXME: Is really needed? Perhaps we should instead take
+ * advantage of the AddtionalReferences parameter to add the
+ * bias when required. This might be the source of the mysterious
+ * ReactOS bug where ObInsertObject *requires* an immediate dereference
+ * even in a success case.
+ * Whill have to think more about this when doing the Increment/Create
+ * split later.
+ */
+ ObReferenceObject(Object);
+ }
+
+ /* Check if we have an open procedure */
+#if 0
+ if (ObjectType->TypeInfo.OpenProcedure)
+ {
+ /* Call it */
+ ObjectType->TypeInfo.OpenProcedure(OpenReason,
+ PsGetCurrentProcess(),
+ Object,
+ AccessState->
+ PreviouslyGrantedAccess,
+ 0);
+ }
+#endif
+
+ /* Increase total number of handles */
+ ObjectType->TotalNumberOfHandles++;
+
+ /* Save the object header (assert its validity too) */
ASSERT((ULONG_PTR)ObjectHeader & EX_HANDLE_ENTRY_LOCKED);
-
- GrantedAccess = AccessState->RemainingDesiredAccess |
- AccessState->PreviouslyGrantedAccess;
- if (GrantedAccess & MAXIMUM_ALLOWED)
- {
- GrantedAccess &= ~MAXIMUM_ALLOWED;
- GrantedAccess |= GENERIC_ALL;
- }
-
- if (GrantedAccess & GENERIC_ACCESS)
- {
- RtlMapGenericMask(&GrantedAccess,
- &ObjectHeader->Type->TypeInfo.GenericMapping);
- }
-
NewEntry.Object = ObjectHeader;
- if(HandleAttributes & OBJ_INHERIT)
- NewEntry.ObAttributes |= EX_HANDLE_ENTRY_INHERITABLE;
- else
- NewEntry.ObAttributes &= ~EX_HANDLE_ENTRY_INHERITABLE;
- NewEntry.GrantedAccess = GrantedAccess;
-
- if ((HandleAttributes & OBJ_KERNEL_HANDLE) &&
- ExGetPreviousMode == KernelMode)
- {
- Process = PsInitialSystemProcess;
- if (Process != CurrentProcess)
- {
- KeStackAttachProcess(&Process->Pcb,
- &ApcState);
- AttachedToProcess = TRUE;
- }
- }
- else
- {
- Process = CurrentProcess;
- /* mask out the OBJ_KERNEL_HANDLE attribute */
- HandleAttributes &= ~OBJ_KERNEL_HANDLE;
- }
-
- Handle = ExCreateHandle(Process->ObjectTable,
- &NewEntry);
-
- if (AttachedToProcess)
- {
- KeUnstackDetachProcess(&ApcState);
- }
-
- if(Handle != NULL)
- {
+
+ /* Mask out the internal attributes */
+ NewEntry.ObAttributes |= HandleAttributes &
+ (EX_HANDLE_ENTRY_PROTECTFROMCLOSE |
+ EX_HANDLE_ENTRY_INHERITABLE |
+ EX_HANDLE_ENTRY_AUDITONCLOSE);
+
+ /* Save the access mask */
+ NewEntry.GrantedAccess = AccessState->RemainingDesiredAccess |
+ AccessState->PreviouslyGrantedAccess;
+
+ /*
+ * Create the actual handle. We'll need to do this *after* calling
+ * ObpIncrementHandleCount to make sure that Object Security is valid
+ * (specified in Gl00my documentation on Ob)
+ */
+ OBTRACE("OBTRACE - %s - Handle Properties: [%p-%lx-%lx]\n",
+ __FUNCTION__,
+ NewEntry.Object, NewEntry.ObAttributes & 3, NewEntry.GrantedAccess);
+ Handle = ExCreateHandle(HandleTable, &NewEntry);
+
+ /* Detach it needed */
+ if (AttachedToProcess) KeUnstackDetachProcess(&ApcState);
+
+ /* Check if we got a handle */
+ if(Handle)
+ {
+ /* Handle extra references */
+ while (AdditionalReferences--)
+ {
+ /* Increment the count */
+ InterlockedIncrement(&ObjectHeader->PointerCount);
+ }
+
+ /* Check if this was a kernel handle */
if (HandleAttributes & OBJ_KERNEL_HANDLE)
{
- /* mark the handle value */
+ /* Set the kernel handle bit */
Handle = ObMarkHandleAsKernelHandle(Handle);
}
- if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
- {
- ObReferenceObject(ObjectBody);
- }
-
+ /* Return handle and object */
*ReturnedHandle = Handle;
-
+ if (ReturnedObject) *ReturnedObject = Object;
+
+ /* Return success */
+ OBTRACE("OBTRACE - %s - Incremented count for: %p. Reason: %lx HC LC %lx %lx\n",
+ __FUNCTION__,
+ Object,
+ OpenReason,
+ ObjectHeader->HandleCount,
+ ObjectHeader->PointerCount);
return STATUS_SUCCESS;
}
- return STATUS_UNSUCCESSFUL;
+ /* Decrement the handle count and fail */
+ ObpDecrementHandleCount(&ObjectHeader->Body,
+ PsGetCurrentProcess(),
+ NewEntry.GrantedAccess);
+ return STATUS_INSUFFICIENT_RESOURCES;
}
/*++
More information about the Ros-diffs
mailing list