On 28/09/2015 11:01, sginsb...@svn.reactos.org wrote: > Author: sginsberg > Date: Mon Sep 28 09:01:11 2015 > New Revision: 69393 > > URL: http://svn.reactos.org/svn/reactos?rev=69393&view=rev > Log: > [NTOS] Fix the Ob wait system calls to only catch the exceptions that are > expected to be raised by the Ke wait functions (and not potentially silently > catching *any* exception and corrupting everything in the process). Also > fixup some code logic. SEH Mega Fixup 1/??? > > Modified: > trunk/reactos/ntoskrnl/ob/obwait.c > > Modified: trunk/reactos/ntoskrnl/ob/obwait.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/obwait.c?rev=69393&r1=69392&r2=69393&view=diff > ============================================================================== > --- trunk/reactos/ntoskrnl/ob/obwait.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/ob/obwait.c [iso-8859-1] Mon Sep 28 > 09:01:11 2015 > @@ -49,12 +49,12 @@ > IN BOOLEAN Alertable, > IN PLARGE_INTEGER TimeOut OPTIONAL) > { > - PKWAIT_BLOCK WaitBlockArray = NULL; > + PKWAIT_BLOCK WaitBlockArray; > HANDLE Handles[MAXIMUM_WAIT_OBJECTS], KernelHandle; > PVOID Objects[MAXIMUM_WAIT_OBJECTS]; > PVOID WaitObjects[MAXIMUM_WAIT_OBJECTS]; > - ULONG i = 0, ReferencedObjects = 0, j; > - KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); > + ULONG i, ReferencedObjects, j; > + KPROCESSOR_MODE PreviousMode; > LARGE_INTEGER SafeTimeOut; > BOOLEAN LockInUse; > PHANDLE_TABLE_ENTRY HandleEntry; > @@ -65,31 +65,26 @@ > NTSTATUS Status; > PAGED_CODE(); > > - /* Enter a critical region since we'll play with handles */ > - LockInUse = TRUE; > - KeEnterCriticalRegion(); > - > /* Check for valid Object Count */ > if ((ObjectCount > MAXIMUM_WAIT_OBJECTS) || !(ObjectCount)) > { > /* Fail */ > - Status = STATUS_INVALID_PARAMETER_1; > - goto Quickie; > + return STATUS_INVALID_PARAMETER_1; > } > > /* Check for valid Wait Type */ > if ((WaitType != WaitAll) && (WaitType != WaitAny)) > { > /* Fail */ > - Status = STATUS_INVALID_PARAMETER_3; > - goto Quickie; > - } > - > - /* Enter SEH */ > - _SEH2_TRY > - { > - /* Check if the call came from user mode */ > - if (PreviousMode != KernelMode) > + return STATUS_INVALID_PARAMETER_3; > + } > + > + /* Enter SEH for user mode */ > + PreviousMode = ExGetPreviousMode(); > + if (PreviousMode != KernelMode) > + { > + /* Enter SEH */ > + _SEH2_TRY
No, this is plain wrong. This is not because you're in kernel mode that the world is marvelous and callers trustable. A caller can pass you buggy address and you HAVE to wrap the RtlCopyMemory in SEH to make sure that if a buggy address is passed, the whole system isn't brought down (that's the whole purpose of the copy after all!). In case you have a doubt, just put some random: Status = ZwWaitForMultipleObjects(2, (void **)0x42424242, WaitAll, FALSE, NULL); In a kernel component. In w2k3, you'll get Status = STATUS_ACCESS_VIOLATION In ReactOS, with your changes: BSOD. Please before doing random changes that you believe are right: do testing. Alex already told you that. Cheers, -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
