[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 do
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=2... ============================================================================== --- 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; }
/*++
-
ion＠svn.reactos.org