Make default queue sequential to fix EvtIoRead

Signed-off-by: Vadym Hrynchyshyn <vadimgrn@gmail.com>
vadimgrn · Jan 8, 2025 · 6813089 · 6813089
1 parent b01c27c
commit 6813089
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 13 deletions.
diff --git a/codeql.txt → doc/codeql.txt b/codeql.txt → doc/codeql.txt
@@ -13,7 +13,7 @@ codeql pack download microsoft/windows-drivers@1.1.0
 rmdir /S /Q D:\codeql-home\databases
 D:\codeql-home\codeql\codeql database create D:\codeql-home\databases --language=cpp --source-root=D:\usbip-win2\drivers --command="msbuild /t:rebuild D:\usbip-win2\usbip_win2.sln"
 
-D:\codeql-home\codeql\codeql database analyze --download D:\codeql-home\databases D:\codeql-home\windows_driver_recommended.qls --format=sarifv2.1.0 --output=D:\usbip-win2\drivers\ude\result.sarif
+codeql database analyze --download ..\databases ..\windows_driver_recommended.qls --format=sarifv2.1.0 --output=D:\usbip-win2\drivers\ude\result.sarif
 
 # Install Visual Studio extention
 # Main menu, Extentions/Manage Extentions, Microsoft SARIF Viewer

diff --git a/doc/whlk.md b/doc/whlk.md
@@ -0,0 +1,3 @@
+- DF - Fuzz zero length buffer IOCTL test (Reliability)
+  - The test has been running for more than four hours. There are no errors, but it fails due to unexpectedly long execution time. The test timeout is 180 minutes.
+  - To fix that, set MinFunctionCode=2048 and MaxFunctionCode=2053 parameters to specify the range of IOCTL function codes used in the calls. See include\usbip\vhci.h, usbip::vhci::ioctl::function.
diff --git a/drivers/ude/context.h b/drivers/ude/context.h
@@ -46,8 +46,6 @@ constexpr auto is_valid_port(int port)
  */
 struct vhci_ctx
 {
-        WDFQUEUE sequential_queue; // see also WdfDeviceGetDefaultQueue
-
         UDECXUSBDEVICE devices[TOTAL_PORTS]; // do not access directly, functions must be used
         WDFSPINLOCK devices_lock;
 

diff --git a/drivers/ude/vhci_ioctl.cpp b/drivers/ude/vhci_ioctl.cpp
@@ -789,9 +789,9 @@ PAGED void device_control_parallel(
                 st = handler(Request);
         } else {
                 auto vhci = WdfIoQueueGetDevice(Queue);
-                auto ctx = get_vhci_ctx(vhci);
+                auto def_queue = WdfDeviceGetDefaultQueue(vhci);
 
-                st = WdfRequestForwardToIoQueue(Request, ctx->sequential_queue); // -> device_control
+                st = WdfRequestForwardToIoQueue(Request, def_queue); // -> device_control
 
                 if (NT_SUCCESS(st)) [[likely]] {
                         st = STATUS_PENDING;
@@ -861,23 +861,25 @@ PAGED NTSTATUS usbip::vhci::create_queues(_In_ WDFDEVICE vhci)
         attr.ParentObject = vhci;
 
         WDF_IO_QUEUE_CONFIG cfg;
-        WDF_IO_QUEUE_CONFIG_INIT_DEFAULT_QUEUE(&cfg, WdfIoQueueDispatchParallel); // see WdfDeviceGetDefaultQueue
+        WDF_IO_QUEUE_CONFIG_INIT_DEFAULT_QUEUE(&cfg, WdfIoQueueDispatchSequential); // WdfDeviceGetDefaultQueue
         cfg.PowerManaged = PowerManaged;
-        cfg.EvtIoDeviceControl = device_control_parallel;
+        cfg.EvtIoDeviceControl = device_control;
         cfg.EvtIoRead = device_read;
 
         if (auto err = WdfIoQueueCreate(vhci, &cfg, &attr, nullptr)) {
-                Trace(TRACE_LEVEL_ERROR, "WdfIoQueueCreate(parallel) %!STATUS!", err);
+                Trace(TRACE_LEVEL_ERROR, "WdfIoQueueCreate %!STATUS!", err);
                 return err;
         }
 
-        WDF_IO_QUEUE_CONFIG_INIT(&cfg, WdfIoQueueDispatchSequential);
+        WDF_IO_QUEUE_CONFIG_INIT(&cfg, WdfIoQueueDispatchParallel);
         cfg.PowerManaged = PowerManaged;
-        cfg.EvtIoDeviceControl = device_control;
+        cfg.EvtIoDeviceControl = device_control_parallel;
 
-        if (auto ctx = get_vhci_ctx(vhci);
-            auto err = WdfIoQueueCreate(vhci, &cfg, &attr, &ctx->sequential_queue)) {
-                Trace(TRACE_LEVEL_ERROR, "WdfIoQueueCreate(sequential) %!STATUS!", err);
+        if (WDFQUEUE queue{}; auto err = WdfIoQueueCreate(vhci, &cfg, &attr, &queue)) {
+                Trace(TRACE_LEVEL_ERROR, "WdfIoQueueCreate %!STATUS!", err);
+                return err;
+        } else if (err = WdfDeviceConfigureRequestDispatching(vhci, queue, WdfRequestTypeDeviceControl); err) {
+                Trace(TRACE_LEVEL_ERROR, "WdfDeviceConfigureRequestDispatching %!STATUS!", err);
                 return err;
         }