[tfaber] 66304: [NTOS] - In ExpReleaseOrWaitForKeyedEvent, always restore previous values for ETHREAD::KeyedWaitValue and ETHREAD::KeyedWaitChain, as these fields have alternative meanings outside ...

Author: tfaber
Date: Sun Feb 15 22:20:34 2015
New Revision: 66304

URL: http://svn.reactos.org/svn/reactos?rev=66304&view=rev
Log:
[NTOS]
- In ExpReleaseOrWaitForKeyedEvent, always restore previous values for ETHREAD::KeyedWaitValue and ETHREAD::KeyedWaitChain, as these fields have alternative meanings outside of keyed events
- Add missing list walk in ExpReleaseOrWaitForKeyedEvent
- In Nt(WaitFor|Release)KeyedEvent, refuse keys that are not two-byte aligned as shown by Wine tests
- Don't forget to set KTHREAD::Process on thread creation
Fixes hang when running ntdll_winetest:om as well as failing tests.
ROSTESTS-118 #resolve

Modified:
    trunk/reactos/ntoskrnl/ex/keyedevt.c
    trunk/reactos/ntoskrnl/ke/thrdobj.c
    trunk/rostests/winetests/ntdll/om.c

Modified: trunk/reactos/ntoskrnl/ex/keyedevt.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/keyedevt.c?rev=...
==============================================================================
--- trunk/reactos/ntoskrnl/ex/keyedevt.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ex/keyedevt.c	[iso-8859-1] Sun Feb 15 22:20:34 2015
@@ -132,6 +132,7 @@
     PLIST_ENTRY ListEntry, WaitListHead1, WaitListHead2;
     NTSTATUS Status;
     ULONG_PTR HashIndex;
+    PVOID PreviousKeyedWaitValue;

/* Get the current process */
     CurrentProcess = KeGetCurrentProcess();
@@ -167,6 +168,7 @@
            be signaled by this thread or, when the wait is aborted due to thread
            termination, then it first needs to acquire the list lock. */
         Thread = CONTAINING_RECORD(ListEntry, ETHREAD, KeyedWaitChain);
+        ListEntry = ListEntry->Flink;

/* Check if this thread is a correct waiter */
         if ((Thread->Tcb.Process == CurrentProcess) &&
@@ -179,7 +181,10 @@
             InitializeListHead(&Thread->KeyedWaitChain);

/* Wake the thread */
-            KeReleaseSemaphore(&Thread->KeyedWaitSemaphore, 0, 1, FALSE);
+            KeReleaseSemaphore(&Thread->KeyedWaitSemaphore,
+                               IO_NO_INCREMENT,
+                               1,
+                               FALSE);
             Thread = NULL;

/* Unlock the list. After this it is not safe to access Thread */
@@ -193,7 +198,8 @@
     /* Get the current thread */
     CurrentThread = PsGetCurrentThread();

-    /* Set the wait key */
+    /* Set the wait key and remember the old value */
+    PreviousKeyedWaitValue = CurrentThread->KeyedWaitValue;
     CurrentThread->KeyedWaitValue = KeyedWaitValue;

/* Initialize the wait semaphore */
@@ -221,16 +227,20 @@
         ExAcquirePushLockExclusive(&KeyedEvent->HashTable[HashIndex].Lock);

/* Check if the wait list entry is still in the list */
-        if (CurrentThread->KeyedWaitChain.Flink != &CurrentThread->KeyedWaitChain)
+        if (!IsListEmpty(&CurrentThread->KeyedWaitChain))
         {
             /* Remove the thread from the list */
             RemoveEntryList(&CurrentThread->KeyedWaitChain);
+            InitializeListHead(&CurrentThread->KeyedWaitChain);
         }

/* Unlock the list */
         ExReleasePushLockExclusive(&KeyedEvent->HashTable[HashIndex].Lock);
         KeLeaveCriticalRegion();
     }
+
+    /* Restore the previous KeyedWaitValue, since this is a union member */
+    CurrentThread->KeyedWaitValue = PreviousKeyedWaitValue;

return Status;
 }
@@ -412,6 +422,12 @@
     NTSTATUS Status;
     LARGE_INTEGER TimeoutCopy;

+    /* Key must always be two-byte aligned */
+    if ((ULONG_PTR)Key & 1)
+    {
+        return STATUS_INVALID_PARAMETER_1;
+    }
+
     /* Check if the caller passed a timeout value and this is from user mode */
     if ((Timeout != NULL) && (PreviousMode != KernelMode))
     {
@@ -472,6 +488,12 @@
     NTSTATUS Status;
     LARGE_INTEGER TimeoutCopy;

+    /* Key must always be two-byte aligned */
+    if ((ULONG_PTR)Key & 1)
+    {
+        return STATUS_INVALID_PARAMETER_1;
+    }
+
     /* Check if the caller passed a timeout value and this is from user mode */
     if ((Timeout != NULL) && (PreviousMode != KernelMode))
     {

Modified: trunk/reactos/ntoskrnl/ke/thrdobj.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/thrdobj.c?rev=6...
==============================================================================
--- trunk/reactos/ntoskrnl/ke/thrdobj.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/thrdobj.c	[iso-8859-1] Sun Feb 15 22:20:34 2015
@@ -838,8 +838,9 @@
     TimerWaitBlock->WaitListEntry.Flink = &Timer->Header.WaitListHead;
     TimerWaitBlock->WaitListEntry.Blink = &Timer->Header.WaitListHead;

-    /* Set the TEB */
+    /* Set the TEB and process */
     Thread->Teb = Teb;
+    Thread->Process = Process;

/* Check if we have a kernel stack */
     if (!KernelStack)

Modified: trunk/rostests/winetests/ntdll/om.c
URL: http://svn.reactos.org/svn/reactos/trunk/rostests/winetests/ntdll/om.c?rev=6...
==============================================================================
--- trunk/rostests/winetests/ntdll/om.c	[iso-8859-1] (original)
+++ trunk/rostests/winetests/ntdll/om.c	[iso-8859-1] Sun Feb 15 22:20:34 2015
@@ -1080,8 +1080,5 @@
     test_query_object();
     test_type_mismatch();
     test_event();
-    if (winetest_interactive)
-        test_keyed_events();
-    else
-        skip("Skipping test_keyed_events(). ROSTESTS-118.\n");
-}
+    test_keyed_events();
+}