[ros-diffs] [arty] 35581: Catch failure to release the cancel spinlock. Would've spotted my error in tcpip.sys.
Alex Ionescu
ionucu at videotron.ca
Sun Aug 24 02:03:32 CEST 2008
--- Please, please, don't think I'm trying to say "revert this!" ---
There are dozens (if not more) places in the kernel where such checks
could/should be made. I believe the more correct approach instead of
adding them to every function, is to use a similar framework to
Microsoft's Driver Verifier, where such functions are hooked and
wrapped through similar verifications.
I'm not saying this particular assert should be removed, I'm just
letting people know that if there's going to be major work on this --
there's a better way.
Just my 2c.
On 23-Aug-08, at 4:04 PM, arty at svn.reactos.org wrote:
> Author: arty
> Date: Sat Aug 23 18:04:15 2008
> New Revision: 35581
>
> URL: http://svn.reactos.org/svn/reactos?rev=35581&view=rev
> Log:
> Catch failure to release the cancel spinlock. Would've spotted my
> error in
> tcpip.sys.
>
> Modified:
> trunk/reactos/ntoskrnl/io/iomgr/irp.c
>
> Modified: trunk/reactos/ntoskrnl/io/iomgr/irp.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/irp.c?rev=35581&r1=35580&r2=35581&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/ntoskrnl/io/iomgr/irp.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/io/iomgr/irp.c [iso-8859-1] Sat Aug 23
> 18:04:15 2008
> @@ -970,12 +970,14 @@
> IoCancelIrp(IN PIRP Irp)
> {
> KIRQL OldIrql;
> + KIRQL IrqlAtEntry;
> PDRIVER_CANCEL CancelRoutine;
> IOTRACE(IO_IRP_DEBUG,
> "%s - Canceling IRP %p\n",
> __FUNCTION__,
> Irp);
> ASSERT(Irp->Type == IO_TYPE_IRP);
> + IrqlAtEntry = KeGetCurrentIrql();
>
> /* Acquire the cancel lock and cancel the IRP */
> IoAcquireCancelSpinLock(&OldIrql);
> @@ -999,6 +1001,7 @@
> /* Set the cancel IRQL And call the routine */
> Irp->CancelIrql = OldIrql;
> CancelRoutine(IoGetCurrentIrpStackLocation(Irp)-
> >DeviceObject, Irp);
> + ASSERT(IrqlAtEntry == KeGetCurrentIrql());
> return TRUE;
> }
>
>
Best regards,
Alex Ionescu
More information about the Ros-diffs
mailing list