[ros-diffs] [ion] 22706: - Add more tracing/name decoration/comments. - Bugcheck if cancelling and IRP that's already been completed. - Bugcheck if attempting to call a driver with an IRP that's already past its maximum stack size. - Make sure that when we free an IRP, it's not associated to a thread anymore, nor having any drivers that think it's valid.

Author: ion
Date: Fri Jun 30 08:29:32 2006
New Revision: 22706

URL: http://svn.reactos.org/svn/reactos?rev=22706&view=rev
Log:
- Add more tracing/name decoration/comments.
- Bugcheck if cancelling and IRP that's already been completed.
- Bugcheck if attempting to call a driver with an IRP that's already past its maximum
stack size.
- Make sure that when we free an IRP, it's not associated to a thread anymore, nor
having any drivers that think it's valid.

Modified:
    trunk/reactos/ntoskrnl/io/irp.c

Modified: trunk/reactos/ntoskrnl/io/irp.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/irp.c?rev=2270…
==============================================================================
--- trunk/reactos/ntoskrnl/io/irp.c (original)
+++ trunk/reactos/ntoskrnl/io/irp.c Fri Jun 30 08:29:32 2006
@@ -13,6 +13,10 @@
 #include <ntoskrnl.h>
 #define NDEBUG
 #include <internal/debug.h>
+
+/* Undefine some macros we implement here */
+#undef IoCallDriver
+#undef IoCompleteRequest
 
 ULONG IopTraceLevel = IO_IRP_DEBUG;
 
@@ -843,40 +847,61 @@
  */
 BOOLEAN
 NTAPI
-IoCancelIrp(PIRP Irp)
-{
-   KIRQL oldlvl;
-   PDRIVER_CANCEL CancelRoutine;
-
-   DPRINT("IoCancelIrp(Irp 0x%p)\n",Irp);
-
-   IoAcquireCancelSpinLock(&oldlvl);
-
-   Irp->Cancel = TRUE;
-
-   CancelRoutine = IoSetCancelRoutine(Irp, NULL);
-   if (CancelRoutine == NULL)
-   {
-      IoReleaseCancelSpinLock(oldlvl);
-      return(FALSE);
-   }
-
-   Irp->CancelIrql = oldlvl;
-   CancelRoutine(IoGetCurrentIrpStackLocation(Irp)->DeviceObject, Irp);
-   return(TRUE);
-}
-
-/*
- * @name IoCancelThreadIo
+IoCancelIrp(IN PIRP Irp)
+{
+    KIRQL OldIrql;
+    PDRIVER_CANCEL CancelRoutine;
+
+    /* Acquire the cancel lock and cancel the IRP */
+    IOTRACE(IO_IRP_DEBUG,
+            "%s - Canceling IRP %p\n",
+            __FUNCTION__,
+            Irp);
+
+    IoAcquireCancelSpinLock(&OldIrql);
+    Irp->Cancel = TRUE;
+
+    /* Clear the cancel routine and get the old one */
+    CancelRoutine = IoSetCancelRoutine(Irp, NULL);
+    if (CancelRoutine)
+    {
+        /* We had a routine, make sure the IRP isn't completed */
+        if (Irp->CurrentLocation > (Irp->StackCount + 1))
+        {
+            /* It is, bugcheck */
+            KeBugCheckEx(CANCEL_STATE_IN_COMPLETED_IRP,
+                         (ULONG_PTR)Irp,
+                         0,
+                         0,
+                         0);
+        }
+
+        /* Set the cancel IRQL And call the routine */
+        Irp->CancelIrql = OldIrql;
+        CancelRoutine(IoGetCurrentIrpStackLocation(Irp)->DeviceObject, Irp);
+        return TRUE;
+    }
+
+    /* Otherwise, release the cancel lock and fail */
+    IoReleaseCancelSpinLock(OldIrql);
+    return FALSE;
+}
+
+/*
+ * @implemented
  */
 VOID
 NTAPI
-IoCancelThreadIo(PETHREAD Thread)
+IoCancelThreadIo(IN PETHREAD Thread)
 {
     PIRP Irp;
     KIRQL OldIrql;
     ULONG Retries = 3000;
     LARGE_INTEGER Interval;
+    IOTRACE(IO_IRP_DEBUG,
+            "%s - Canceling IRPs for Thread %p\n",
+            __FUNCTION__,
+            Thread);
 
     /* Raise to APC to protect the IrpList */
     OldIrql = KfRaiseIrql(APC_LEVEL);
@@ -904,19 +929,16 @@
          * Don't stay here forever if some broken driver doesn't complete
          * the IRP.
          */
-        if (Retries-- == 0) IopRemoveThreadIrp();
+        if (!(Retries--)) IopRemoveThreadIrp();
 
         /* Raise the IRQL Again */
         OldIrql = KfRaiseIrql(APC_LEVEL);
     }
-    
+
     /* We're done, lower the IRQL */
     KfLowerIrql(OldIrql);
 }
 
-#ifdef IoCallDriver
-#undef IoCallDriver
-#endif
 /*
  * @implemented
  */
@@ -932,7 +954,6 @@
 /*
  * @implemented
  */
-#undef IoCompleteRequest
 VOID
 NTAPI
 IoCompleteRequest(PIRP Irp,
@@ -949,6 +970,7 @@
 NTAPI
 IoEnqueueIrp(IN PIRP Irp)
 {
+    /* This is the same as calling IoQueueThreadIrp */
     IoQueueThreadIrp(Irp);
 }
 
@@ -966,11 +988,17 @@
     /* Get the Driver Object */
     DriverObject = DeviceObject->DriverObject;
 
-    /* Set the Stack Location */
-    IoSetNextIrpStackLocation(Irp);
-
-    /* Get the current one */
-    Param = IoGetCurrentIrpStackLocation(Irp);
+    /* Decrease the current location and check if */
+    Irp->CurrentLocation--;
+    if (Irp->CurrentLocation <= 0)
+    {
+        /* This IRP ran out of stack, bugcheck */
+        KeBugCheckEx(NO_MORE_IRP_STACK_LOCATIONS, (ULONG_PTR)Irp, 0, 0, 0);
+    }
+
+    /* Now update the stack location */
+    Param = IoGetNextIrpStackLocation(Irp);
+    Irp->Tail.Overlay.CurrentStackLocation = Param;
 
     /* Get the Device Object */
     Param->DeviceObject = DeviceObject;
@@ -980,9 +1008,6 @@
                                                              Irp);
 }
 
-#ifdef IoCompleteRequest
-#undef IoCompleteRequest
-#endif
 /*
  * @implemented
  */
@@ -1242,7 +1267,15 @@
     PNPAGED_LOOKASIDE_LIST List;
     PP_NPAGED_LOOKASIDE_NUMBER ListType =  LookasideSmallIrpList;
     PKPRCB Prcb;
-    
+    IOTRACE(IO_IRP_DEBUG,
+            "%s - Freeing IRPs %p\n",
+            __FUNCTION__,
+            Irp);
+
+    /* Make sure the Thread IRP list is empty and that it OK to free it */
+    ASSERT(IsListEmpty(&Irp->ThreadListEntry));
+    ASSERT(Irp->CurrentLocation >= Irp->StackCount);
+
     /* If this was a pool alloc, free it with the pool */
     if (!(Irp->AllocationFlags & IRP_ALLOCATED_FIXED_SIZE))
     {
@@ -1252,10 +1285,7 @@
     else
     {
         /* Check if this was a Big IRP */
-        if (Irp->StackCount != 1)
-        {
-            ListType = LookasideLargeIrpList;
-        }
+        if (Irp->StackCount != 1) ListType = LookasideLargeIrpList;
 
         /* Get the PRCB */
         Prcb = KeGetCurrentPrcb();
@@ -1284,7 +1314,7 @@
             }
         }
 
-        /* The free was within dhe Depth */
+        /* The free was within the Depth */
         if (Irp)
         {
            InterlockedPushEntrySList(&List->L.ListHead,