Do not use invalid queue handle on Rx notification

When system goes to S3 (legacy standby), devices goes from D0 to D3 and
NetAdapter destroys datapath, which destroys Tx/Rx queues. When system
wakes up, NetAdapter recreates queues.

While we update Rx queue handle in EvtAdapterCreateRxQueue, there is
time frame between queue being destroyed and new queue is created.
If we got incoming packet on the socket during that time frame,
we inticate it to Rx queue and white doing that, we get queue's WDF
context. When this is done with the handle of destroyed queue,
this causes NULL pointer access inside WDF framework.

Fix by assigning Rx queue handle to adapter context in EvtQueueStart
callback and settings it to WDF_NO_HANDLE in EvtRxQueueStop.
In OvpnAdapterNotifyRx proceed with Rx notification only if
rxQueue handle is valid.

Fixes #36

Signed-off-by: Lev Stipakov <[email protected]>

Driver.cpp

@@ -73,7 +73,9 @@ DriverEntry(_In_ PDRIVER_OBJECT driverObject, _In_ PUNICODE_STRING registryPath)
     GOTO_IF_NOT_NT_SUCCESS(done, status, TraceLoggingRegister(g_hOvpnEtwProvider));
     traceLoggingRegistered = TRUE;
 
-    LOG_INFO("Driver Version", TraceLoggingValue(OVPN_DCO_VERSION_MAJOR, "Major"), TraceLoggingValue(OVPN_DCO_VERSION_MINOR, "Minor"), TraceLoggingValue(OVPN_DCO_VERSION_PATCH, "Patch"));
+    LOG_INFO("Driver Version", TraceLoggingValue(OVPN_DCO_VERSION_MAJOR, "Major"),
+        TraceLoggingValue(OVPN_DCO_VERSION_MINOR, "Minor"),
+        TraceLoggingValue(OVPN_DCO_VERSION_PATCH, "Patch"));
 
     WDF_OBJECT_ATTRIBUTES driverAttrs;
     WDF_OBJECT_ATTRIBUTES_INIT(&driverAttrs);
@@ -313,6 +315,61 @@ VOID OvpnEvtDeviceCleanup(WDFOBJECT obj) {
     LOG_EXIT();
 }
 
+EVT_WDF_DEVICE_PREPARE_HARDWARE OvpnEvtDevicePrepareHardware;
+EVT_WDF_DEVICE_RELEASE_HARDWARE OvpnEvtDeviceReleaseHardware;
+_No_competing_thread_ EVT_WDF_DEVICE_D0_ENTRY OvpnEvtDeviceD0Entry;
+_No_competing_thread_ EVT_WDF_DEVICE_D0_EXIT OvpnEvtDeviceD0Exit;
+
+_Use_decl_annotations_
+NTSTATUS
+OvpnEvtDevicePrepareHardware(_In_ WDFDEVICE wdfDevice, _In_ WDFCMRESLIST resourcesRaw, _In_ WDFCMRESLIST resourcesTranslated)
+{
+    UNREFERENCED_PARAMETER(wdfDevice);
+    UNREFERENCED_PARAMETER(resourcesRaw);
+    UNREFERENCED_PARAMETER(resourcesTranslated);
+
+    LOG_ENTER();
+    LOG_EXIT();
+    return STATUS_SUCCESS;
+}
+
+_Use_decl_annotations_
+NTSTATUS
+OvpnEvtDeviceReleaseHardware(_In_ WDFDEVICE wdfDevice, _In_ WDFCMRESLIST resourcesTranslated)
+{
+    UNREFERENCED_PARAMETER(wdfDevice);
+    UNREFERENCED_PARAMETER(resourcesTranslated);
+
+    LOG_ENTER();
+    LOG_EXIT();
+    return STATUS_SUCCESS;
+}
+
+_Use_decl_annotations_
+NTSTATUS
+OvpnEvtDeviceD0Entry(_In_ WDFDEVICE wdfDevice, WDF_POWER_DEVICE_STATE previousState)
+{
+    UNREFERENCED_PARAMETER(wdfDevice);
+
+    LOG_ENTER(TraceLoggingUInt32(previousState, "PreviousState"));
+
+    LOG_EXIT();
+
+    return STATUS_SUCCESS;
+}
+
+_Use_decl_annotations_
+NTSTATUS
+OvpnEvtDeviceD0Exit(_In_ WDFDEVICE Device, _In_ WDF_POWER_DEVICE_STATE TargetState)
+{
+    UNREFERENCED_PARAMETER(Device);
+
+    LOG_ENTER(TraceLoggingUInt32(TargetState, "TargetState"));
+
+    LOG_EXIT();
+    return STATUS_SUCCESS;
+}
+
 EVT_WDF_DRIVER_DEVICE_ADD OvpnEvtDeviceAdd;
 
 _Use_decl_annotations_
@@ -335,6 +392,14 @@ OvpnEvtDeviceAdd(WDFDRIVER wdfDriver, PWDFDEVICE_INIT deviceInit) {
     NTSTATUS status;
     GOTO_IF_NOT_NT_SUCCESS(done, status, NetDeviceInitConfig(deviceInit));
 
+    WDF_PNPPOWER_EVENT_CALLBACKS pnpPowerCallbacks;
+    WDF_PNPPOWER_EVENT_CALLBACKS_INIT(&pnpPowerCallbacks);
+    pnpPowerCallbacks.EvtDevicePrepareHardware = OvpnEvtDevicePrepareHardware;
+    pnpPowerCallbacks.EvtDeviceReleaseHardware = OvpnEvtDeviceReleaseHardware;
+    pnpPowerCallbacks.EvtDeviceD0Entry = OvpnEvtDeviceD0Entry;
+    pnpPowerCallbacks.EvtDeviceD0Exit = OvpnEvtDeviceD0Exit;
+    WdfDeviceInitSetPnpPowerEventCallbacks(deviceInit, &pnpPowerCallbacks);
+
     WDF_OBJECT_ATTRIBUTES objAttributes;
     WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&objAttributes, OVPN_DEVICE);
     // BCryptOpenAlgorithmProvider with BCRYPT_PROV_DISPATCH returns STATUS_NOT_SUPPORTED if sync scope is WdfSynchronizationScopeDevice

adapter.cpp

@@ -132,21 +132,28 @@ _Use_decl_annotations_
 NTSTATUS
 OvpnEvtAdapterCreateRxQueue(NETADAPTER netAdapter, NETRXQUEUE_INIT* rxQueueInit)
 {
+    LOG_ENTER();
+
     WDF_OBJECT_ATTRIBUTES rxAttributes;
     NET_PACKET_QUEUE_CONFIG rxConfig;
 
     WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&rxAttributes, OVPN_RXQUEUE);
+    rxAttributes.EvtDestroyCallback = OvpnEvtRxQueueDestroy;
+
     NET_PACKET_QUEUE_CONFIG_INIT(&rxConfig, OvpnEvtRxQueueAdvance, OvpnEvtRxQueueSetNotificationEnabled, OvpnEvtRxQueueCancel);
+    rxConfig.EvtStart = OvpnEvtRxQueueStart;
+    rxConfig.EvtStop = OvpnEvtRxQueueStop;
 
     NETPACKETQUEUE netPacketQueue;
     NTSTATUS status;
     GOTO_IF_NOT_NT_SUCCESS(done, status, NetRxQueueCreate(rxQueueInit, &rxAttributes, &rxConfig, &netPacketQueue));
 
     POVPN_ADAPTER adapterContext = OvpnGetAdapterContext(netAdapter);
     OvpnRxQueueInitialize(netPacketQueue, adapterContext);
-    adapterContext->RxQueue = netPacketQueue;
 
 done:
+    LOG_EXIT();
+
     return status;
 }
 
@@ -207,18 +214,20 @@ OvpnAdapterCreate(OVPN_DEVICE * device) {
     return status;
 }
 
-NTSTATUS OvpnAdapterNotifyRx(NETADAPTER netAdapter)
+VOID OvpnAdapterNotifyRx(NETADAPTER netAdapter)
 {
     if (netAdapter == WDF_NO_HANDLE) {
         LOG_ERROR("Adapter not initialized");
-        return STATUS_DEVICE_NOT_READY;
+        return;
     }
 
     NETPACKETQUEUE rxQueue = OvpnGetAdapterContext(netAdapter)->RxQueue;
-    POVPN_RXQUEUE queueContext = OvpnGetRxQueueContext(rxQueue);
+    if (rxQueue == WDF_NO_HANDLE) {
+        LOG_WARN("rxQueue not initialized");
+        return;
+    }
 
+    POVPN_RXQUEUE queueContext = OvpnGetRxQueueContext(rxQueue);
     if (InterlockedExchange(&queueContext->NotificationEnabled, FALSE) == TRUE)
         NetRxQueueNotifyMoreReceivedPacketsAvailable(rxQueue);
-
-    return STATUS_SUCCESS;
 }

adapter.h

@@ -49,7 +49,7 @@ NTSTATUS
 OvpnAdapterCreate(OVPN_DEVICE* device);
 
 // notify NetAdapter (if it is ready) that more packets are available
-NTSTATUS
+VOID
 OvpnAdapterNotifyRx(NETADAPTER netAdapter);
 
 VOID

rxqueue.cpp

@@ -29,9 +29,42 @@
 #include "bufferpool.h"
 #include "rxqueue.h"
 #include "netringiterator.h"
+#include "trace.h"
 
 EVT_PACKET_QUEUE_ADVANCE OvpnEvtRxQueueAdvance;
 
+_Use_decl_annotations_
+VOID
+OvpnEvtRxQueueStart(NETPACKETQUEUE netPacketQueue)
+{
+    LOG_ENTER(TraceLoggingPointer(netPacketQueue, "RxQueue"));
+
+    POVPN_RXQUEUE queue = OvpnGetRxQueueContext(netPacketQueue);
+    queue->Adapter->RxQueue = netPacketQueue;
+
+    LOG_EXIT();
+}
+
+_Use_decl_annotations_
+VOID
+OvpnEvtRxQueueStop(NETPACKETQUEUE netPacketQueue)
+{
+    LOG_ENTER(TraceLoggingPointer(netPacketQueue, "RxQueue"));
+
+    POVPN_RXQUEUE queue = OvpnGetRxQueueContext(netPacketQueue);
+    queue->Adapter->RxQueue = WDF_NO_HANDLE;
+
+    LOG_EXIT();
+}
+
+_Use_decl_annotations_
+VOID
+OvpnEvtRxQueueDestroy(WDFOBJECT rxQueue)
+{
+    LOG_ENTER(TraceLoggingPointer(rxQueue, "RxQueue"));
+    LOG_EXIT();
+}
+
 static inline UINT8
 OvpnRxQueueGetLayer4Type(const VOID* buf, size_t len)
 {

rxqueue.h

@@ -41,5 +41,10 @@ EVT_PACKET_QUEUE_SET_NOTIFICATION_ENABLED OvpnEvtRxQueueSetNotificationEnabled;
 EVT_PACKET_QUEUE_ADVANCE OvpnEvtRxQueueAdvance;
 EVT_PACKET_QUEUE_CANCEL OvpnEvtRxQueueCancel;
 
+EVT_PACKET_QUEUE_START OvpnEvtRxQueueStart;
+EVT_PACKET_QUEUE_STOP OvpnEvtRxQueueStop;
+
+EVT_WDF_OBJECT_CONTEXT_DESTROY OvpnEvtRxQueueDestroy;
+
 VOID
 OvpnRxQueueInitialize(NETPACKETQUEUE rxQueue, _In_ POVPN_ADAPTER adapter);

trace.h

@@ -65,24 +65,24 @@ TRACELOGGING_DECLARE_PROVIDER(g_hOvpnEtwProvider);
             __VA_ARGS__); \
 } while (0,0)
 
-#define LOG_ENTER() do {\
+#define LOG_ENTER(...) do {\
         TraceLoggingWrite( \
             g_hOvpnEtwProvider, \
-            "Info", \
-            TraceLoggingLevel(TRACE_LEVEL_INFORMATION), \
+            "FunctionEntry", \
+            TraceLoggingLevel(TRACE_LEVEL_VERBOSE), \
             TraceLoggingFunctionName(), \
             TraceLoggingUInt32(__LINE__, "Line"), \
-            TraceLoggingValue("Enter", "Msg")); \
+            __VA_ARGS__); \
 } while (0,0)
 
-#define LOG_EXIT() do {\
+#define LOG_EXIT(...) do {\
         TraceLoggingWrite( \
             g_hOvpnEtwProvider, \
-            "Info", \
-            TraceLoggingLevel(TRACE_LEVEL_INFORMATION), \
+            "FunctionExit", \
+            TraceLoggingLevel(TRACE_LEVEL_VERBOSE), \
             TraceLoggingFunctionName(), \
             TraceLoggingUInt32(__LINE__, "Line"), \
-            TraceLoggingValue("Exit", "Msg")); \
+            __VA_ARGS__); \
 } while (0,0)
 
 #define LOG_INFO(Info, ...) do {\