[ros-dev] KQUEUE race condition

[Filip Navara]

Hi,

I think I found a race condition in KQUEUE code, but I'm not sure my 
solution is optimal. If no items are queued, KeRemoveQueue calls 
KeWaitForSingleObject and then after it finishes it acquires dispatcher 
lock and removes a list item. The problem arises when someone calls 
KeRemoveQueue just after the KeWaitForSingleObject call finishes and 
before the dispatcher lock is acquired. In this case he can remove the 
last item in the list and this will cause the first KeRemoveQueue (the 
one that waited) to return garbage. Can anybody think of better solution 
than the attached patch?

Regards,
Filip

Index: ntoskrnl/ke/queue.c
===================================================================
RCS file: /CVS/ReactOS/reactos/ntoskrnl/ke/queue.c,v
retrieving revision 1.11
diff -u -r1.11 queue.c
--- ntoskrnl/ke/queue.c	15 Aug 2004 16:39:05 -0000	1.11
+++ ntoskrnl/ke/queue.c	21 Nov 2004 00:39:52 -0000
@@ -159,27 +159,38 @@
       return ListEntry;
    }
 
-   //need to wait for it...
-   KeReleaseDispatcherDatabaseLock (OldIrql);
-
-   Status = KeWaitForSingleObject(Queue,
-                                  WrQueue,
-                                  WaitMode,
-                                  TRUE,//Alertable,
-                                  Timeout);
-
-   if (Status == STATUS_TIMEOUT || Status == STATUS_USER_APC)
-   {
-      return (PVOID)Status;
-   }
-   else
+   for (;;)
    {
-      OldIrql = KeAcquireDispatcherDatabaseLock ();
-      ListEntry = RemoveHeadList(&Queue->EntryListHead);
+      //need to wait for it...
       KeReleaseDispatcherDatabaseLock (OldIrql);
-      return ListEntry;
-   }
 
+      Status = KeWaitForSingleObject(Queue,
+                                     WrQueue,
+                                     WaitMode,
+                                     TRUE,//Alertable,
+                                     Timeout);
+
+      if (Status == STATUS_TIMEOUT || Status == STATUS_USER_APC)
+      {
+         return (PVOID)Status;
+      }
+      else
+      {
+         OldIrql = KeAcquireDispatcherDatabaseLock ();
+
+         /*
+          * Prevent a race condition. If someone calls KeRemoveQueue
+          * just after we end up waiting and before acquiring the spin
+          * lock he can get the last item in the list...
+          */
+         if (IsListEmpty(&Queue->EntryListHead))
+            continue;
+
+         ListEntry = RemoveHeadList(&Queue->EntryListHead);
+         KeReleaseDispatcherDatabaseLock (OldIrql);
+         return ListEntry;
+      }
+   }
 }