[ros-diffs] [reactos] 01/01: [NTOS:IO] Use a guarded region in IopQueueIrpToThread.

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2e76fb9fe13436a542cc6…

commit 2e76fb9fe13436a542cc647bd3cd4a016a6690b6
Author:     Thomas Faber <thomas.faber(a)reactos.org>
AuthorDate: Thu Oct 28 10:59:20 2021 -0400
Commit:     Thomas Faber <thomas.faber(a)reactos.org>
CommitDate: Sat Nov 20 14:58:51 2021 -0500

    [NTOS:IO] Use a guarded region in IopQueueIrpToThread.
    
    We're protecting against IopCompleteRequest, which is a special
    kernel APC. So this is a little bit faster than raising the IRQL.
---
 ntoskrnl/include/internal/io_x.h | 17 +++++++++++------
 ntoskrnl/include/internal/ke_x.h | 36 ++++++++++++++++++------------------
 2 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/ntoskrnl/include/internal/io_x.h b/ntoskrnl/include/internal/io_x.h
index 7cf42e9dc02..88b19160448 100644
--- a/ntoskrnl/include/internal/io_x.h
+++ b/ntoskrnl/include/internal/io_x.h
@@ -48,22 +48,27 @@ FORCEINLINE
 VOID
 IopQueueIrpToThread(IN PIRP Irp)
 {
-    KIRQL OldIrql;
+    PETHREAD Thread = Irp->Tail.Overlay.Thread;
 
-    /* Raise to APC Level */
-    KeRaiseIrql(APC_LEVEL, &OldIrql);
+    /* Disable special kernel APCs so we can't race with IopCompleteRequest.
+     * IRP's thread must be the current thread */
+    KeEnterGuardedRegionThread(&Thread->Tcb);
 
     /* Insert it into the list */
-    InsertHeadList(&Irp->Tail.Overlay.Thread->IrpList,
&Irp->ThreadListEntry);
+    InsertHeadList(&Thread->IrpList, &Irp->ThreadListEntry);
 
-    /* Lower irql */
-    KeLowerIrql(OldIrql);
+    /* Leave the guarded region */
+    KeLeaveGuardedRegionThread(&Thread->Tcb);
 }
 
 FORCEINLINE
 VOID
 IopUnQueueIrpFromThread(IN PIRP Irp)
 {
+    /* Special kernel APCs must be disabled so we can't race with
+     * IopCompleteRequest (or because we are called from there) */
+    ASSERT(KeAreAllApcsDisabled());
+
     /* Remove it from the list and reset it */
     if (IsListEmpty(&Irp->ThreadListEntry))
         return;
diff --git a/ntoskrnl/include/internal/ke_x.h b/ntoskrnl/include/internal/ke_x.h
index 7a00fba8f9c..70fa05e1d12 100644
--- a/ntoskrnl/include/internal/ke_x.h
+++ b/ntoskrnl/include/internal/ke_x.h
@@ -28,12 +28,12 @@ KeGetPreviousMode(VOID)
 {                                                                           \
     /* Sanity checks */                                                     \
     ASSERT(KeGetCurrentIrql() <= APC_LEVEL);                                \
-    ASSERT(_Thread == KeGetCurrentThread());                                \
-    ASSERT((_Thread->SpecialApcDisable <= 0) &&                            
\
-           (_Thread->SpecialApcDisable != -32768));                         \
+    ASSERT((_Thread) == KeGetCurrentThread());                              \
+    ASSERT(((_Thread)->SpecialApcDisable <= 0) &&                          
\
+           ((_Thread)->SpecialApcDisable != -32768));                       \
                                                                             \
     /* Disable Special APCs */                                              \
-    _Thread->SpecialApcDisable--;                                           \
+    (_Thread)->SpecialApcDisable--;                                         \
 }
 
 #define KeEnterGuardedRegion()                                              \
@@ -49,14 +49,14 @@ KeGetPreviousMode(VOID)
 {                                                                           \
     /* Sanity checks */                                                     \
     ASSERT(KeGetCurrentIrql() <= APC_LEVEL);                                \
-    ASSERT(_Thread == KeGetCurrentThread());                                \
-    ASSERT(_Thread->SpecialApcDisable < 0);                                 \
+    ASSERT((_Thread) == KeGetCurrentThread());                              \
+    ASSERT((_Thread)->SpecialApcDisable < 0);                               \
                                                                             \
     /* Leave region and check if APCs are OK now */                         \
-    if (!(++_Thread->SpecialApcDisable))                                    \
+    if (!(++(_Thread)->SpecialApcDisable))                                  \
     {                                                                       \
         /* Check for Kernel APCs on the list */                             \
-        if (!IsListEmpty(&_Thread->ApcState.                                \
+        if (!IsListEmpty(&(_Thread)->ApcState.                              \
                          ApcListHead[KernelMode]))                          \
         {                                                                   \
             /* Check for APC Delivery */                                    \
@@ -77,12 +77,12 @@ KeGetPreviousMode(VOID)
 #define KeEnterCriticalRegionThread(_Thread)                                \
 {                                                                           \
     /* Sanity checks */                                                     \
-    ASSERT(_Thread == KeGetCurrentThread());                                \
-    ASSERT((_Thread->KernelApcDisable <= 0) &&                             
\
-           (_Thread->KernelApcDisable != -32768));                          \
+    ASSERT((_Thread) == KeGetCurrentThread());                              \
+    ASSERT(((_Thread)->KernelApcDisable <= 0) &&                           
\
+           ((_Thread)->KernelApcDisable != -32768));                        \
                                                                             \
     /* Disable Kernel APCs */                                               \
-    _Thread->KernelApcDisable--;                                            \
+    (_Thread)->KernelApcDisable--;                                          \
 }
 
 #define KeEnterCriticalRegion()                                             \
@@ -97,18 +97,18 @@ KeGetPreviousMode(VOID)
 #define KeLeaveCriticalRegionThread(_Thread)                                \
 {                                                                           \
     /* Sanity checks */                                                     \
-    ASSERT(_Thread == KeGetCurrentThread());                                \
-    ASSERT(_Thread->KernelApcDisable < 0);                                  \
+    ASSERT((_Thread) == KeGetCurrentThread());                              \
+    ASSERT((_Thread)->KernelApcDisable < 0);                                \
                                                                             \
     /* Enable Kernel APCs */                                                \
-    _Thread->KernelApcDisable++;                                            \
+    (_Thread)->KernelApcDisable++;                                          \
                                                                             \
     /* Check if Kernel APCs are now enabled */                              \
-    if (!(_Thread->KernelApcDisable))                                       \
+    if (!((_Thread)->KernelApcDisable))                                     \
     {                                                                       \
         /* Check if we need to request an APC Delivery */                   \
-        if (!(IsListEmpty(&_Thread->ApcState.ApcListHead[KernelMode])) && 
 \
-            !(_Thread->SpecialApcDisable))                                  \
+        if (!(IsListEmpty(&(_Thread)->ApcState.ApcListHead[KernelMode]))
&& \
+            !((_Thread)->SpecialApcDisable))                                \
         {                                                                   \
             /* Check for the right environment */                           \
             KiCheckForKernelApcDelivery();                                  \