[PATCH v2 0/2] MR1559: ntoskrnl.exe: Prevent blocking if kernel timer is reset in DPC.

[Ivo Ivanov (＠logos128)]

KeCancelTimer (which is called via KeSetTimerEx) would block indefinitely in the DPC, while waiting for the timer to complete. Which never happens, since the internal thread pool implementation never gets the chance to wake up the "finished_event" condition variable on the DPC exit in tp_object_execute().

--
  v2: ntoskrnl.exe/tests: Test resetting a kernel timer in its DPC.
      ntoskrnl.exe: Prevent blocking if timer is reset in DPC.

https://gitlab.winehq.org/wine/wine/-/merge_requests/1559

[PATCH v2 1/2] ntoskrnl.exe: Prevent blocking if timer is reset in DPC.

[Ivo Ivanov]

From: Ivo Ivanov

Fixes NaturalPoint's TrackIR5 app hanging indefinitely on exit,
while waiting synchronously on an IRP to complete. The blocking
happens in the app's npusbio_x64 driver, while it tries to reset
a timer in its DPC.
Probably fixes other drivers/apps in such situations.
---
 dlls/ntoskrnl.exe/sync.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/dlls/ntoskrnl.exe/sync.c b/dlls/ntoskrnl.exe/sync.c
index d9b5726b920..0b101158eb6 100644
--- a/dlls/ntoskrnl.exe/sync.c
+++ b/dlls/ntoskrnl.exe/sync.c
@@ -493,10 +493,14 @@ BOOLEAN WINAPI KeSetTimerEx( KTIMER *timer, LARGE_INTEGER duetime, LONG period,

EnterCriticalSection( &sync_cs );

-    if ((ret = timer->Header.Inserted))
-        KeCancelTimer(timer);
-
+    ret = timer->Header.Inserted;
     timer->Header.Inserted = TRUE;
+    timer->Header.SignalState = FALSE;
+    if (timer->Header.WaitListHead.Blink && !*((ULONG_PTR *)&timer->Header.WaitListHead.Flink))
+    {
+        CloseHandle(timer->Header.WaitListHead.Blink);
+        timer->Header.WaitListHead.Blink = NULL;
+    }

if (!timer->TimerListEntry.Blink)
         timer->TimerListEntry.Blink = (void *)CreateThreadpoolTimer(ke_timer_complete_proc, timer, NULL);

[PATCH v2 2/2] ntoskrnl.exe/tests: Test resetting a kernel timer in its DPC.

[Ivo Ivanov]

From: Ivo Ivanov

---
 dlls/ntoskrnl.exe/tests/driver.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c
index d293889c823..dfdca2e24ca 100644
--- a/dlls/ntoskrnl.exe/tests/driver.c
+++ b/dlls/ntoskrnl.exe/tests/driver.c
@@ -578,6 +578,14 @@ static void WINAPI test_sync_dpc(KDPC *dpc, void *context, void *system_argument
     c->called = TRUE;
 }

+static void WINAPI test_sync_dpc_2(KDPC *dpc, void *context, void *system_argument1, void *system_argument2)
+{
+    KTIMER *timer = context;
+    LARGE_INTEGER timeout = {.QuadPart = -20 * 10000};
+
+    KeSetTimerEx(timer, timeout, 0, dpc);
+}
+
 static void test_sync(void)
 {
     static const ULONG wine_tag = 0x454e4957; /* WINE */
@@ -591,7 +599,7 @@ static void test_sync(void)
     void *objs[2];
     KTIMER timer;
     NTSTATUS ret;
-    KDPC dpc;
+    KDPC dpc, dpc_2;
     int i;

KeInitializeEvent(&manual_event, NotificationEvent, FALSE);
@@ -925,6 +933,17 @@ static void test_sync(void)
     ret = wait_single(&timer, -40 * 10000);
     ok(ret == WAIT_TIMEOUT, "got %#lx\n", ret);

+    KeCancelTimer(&timer);
+    /* Test re-setting timer in dpc */
+    KeInitializeDpc(&dpc_2, test_sync_dpc_2, &timer);
+    KeSetTimerEx(&timer, timeout, 0, &dpc_2);
+    ret = wait_single(&timer, -40 * 10000);
+    ok(ret == 0, "got %#lx\n", ret);
+    ret = wait_single(&timer, -40 * 10000);
+    ok(ret == 0, "got %#lx\n", ret);
+    ret = wait_single(&timer, -40 * 10000);
+    ok(ret == 0, "got %#lx\n", ret);
+
     KeCancelTimer(&timer);
     /* Test reinitializing timer. */
     KeSetTimerEx(&timer, timeout, 0, &dpc);