From bb72b1da28e5681a8e4861680ee68c1588d99fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20H=C3=B6glinger-Stelzer?= Date: Thu, 2 Jul 2026 21:27:06 +0200 Subject: [PATCH] PnP Proxy Create/Destroy Guards --- src/Dmf_BusFilter.c | 437 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 336 insertions(+), 101 deletions(-) diff --git a/src/Dmf_BusFilter.c b/src/Dmf_BusFilter.c index 431fe2a..23c2fe8 100644 --- a/src/Dmf_BusFilter.c +++ b/src/Dmf_BusFilter.c @@ -35,61 +35,87 @@ Environment: #if defined(DMF_KERNEL_MODE) // WDM child device context -// +// +// LIFETIME NOTES +// -------------- +// Each WDM filter device object created by this library lives between +// IoAttachDeviceToDeviceStackSafe (end of Relations_AddDevice) and +// IoDeleteDevice (end of Relations_RemoveDevice or DeviceCleanup). +// The mandatory teardown sequence is: +// 1. IoReleaseRemoveLockAndWait -- drain all in-flight I/O holders +// 2. IoDetachDevice -- detach filter from the PDO stack +// 3. IoDeleteDevice -- lazy-free the device object (actual +// memory release deferred to last reference) +// NOTHING may touch DeviceExtension after IoDeleteDevice returns. +// typedef struct _WDM_CHILD_DEVICE_EXTENSION { // GUID to identify WDM child device. - // + // GUID Signature; - // Target Device Object + // Remove lock serialises in-flight I/O against IRP_MN_REMOVE_DEVICE teardown. + // Initialized in Relations_AddDevice; released-and-waited in Relations_RemoveDevice + // (normal path) or DeviceCleanup (force path). + // + IO_REMOVE_LOCK RemoveLock; + + // Target Device Object (result of IoAttachDeviceToDeviceStackSafe) // PDEVICE_OBJECT TargetDeviceObject; - // Physical Device Object + // Physical Device Object (PDO of the bus child being filtered) // PDEVICE_OBJECT PhysicalDeviceObject; // Parent ChildList entry - // + // LIST_ENTRY ListEntry; // Parent WDF device object - // + // WDFDEVICE Parent; // Child WDF wrapper object - // + // DMFBUSCHILDDEVICE Child; - // TRUE if PDO is attached, FALSE otherwise - // + // TRUE if PDO appeared in the most recent bus-relations response + // BOOLEAN IsExisting; } WDM_CHILD_DEVICE_EXTENSION; // Parent bus device context -// +// typedef struct _PARENT_BUS_DEVICE_CONTEXT { - // // List of child device (relations) - // + // LIST_ENTRY ChildList; + // Spin lock protecting ChildList access // - // Spin lock protecting child list access - // KSPIN_LOCK ChildListLock; + + // Count of bus-relations work items currently queued or executing. + // Used to prevent DeviceCleanup from racing a pending passive-level + // AddDevice worker. + // + volatile LONG OutstandingWorkItems; + + // Signaled when OutstandingWorkItems drops to zero (initially signaled). + // + KEVENT WorkItemsIdle; } PARENT_BUS_DEVICE_CONTEXT; WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(PARENT_BUS_DEVICE_CONTEXT, DMF_BusFilter_GetParentContext) // Bus child device context -// +// typedef struct _BUS_CHILD_DEVICE_CONTEXT { // WDM device object - // + // PDEVICE_OBJECT DeviceObject; } BUS_CHILD_DEVICE_CONTEXT; @@ -106,19 +132,19 @@ EVT_DMF_BusFilter_DispatchPnp( ); // Module-internal context data -// +// typedef struct _DMF_CONTEXT_BusFilter { // Copy of the module configuration - // + // DMF_BusFilter_CONFIG Configuration; // Hooked dispatch table - // + // PDRIVER_DISPATCH MajorDispatchFunctions[IRP_MJ_MAXIMUM_FUNCTION + 1]; // PNP minor functions dispatch routines - // + // EVT_DMF_BusFilter_DispatchPnp* PnPMinorDispatchFunctions[IRP_MN_DEVICE_ENUMERATED + 1]; } BusFilter_Context; @@ -134,7 +160,7 @@ DEFINE_GUID(GUID_DMF_BUSFILTER_SIGNATURE, #define DMF_BUSFILTER_POOL_TAG 'tlfB' // Context passed to the passive-level work item used when the bus-relations -// completion routine fires at DISPATCH_LEVEL (Finding 3). +// completion routine fires at DISPATCH_LEVEL. // typedef struct _BUS_RELATIONS_WORK_CONTEXT { @@ -143,12 +169,15 @@ typedef struct _BUS_RELATIONS_WORK_CONTEXT PIRP Irp; } BUS_RELATIONS_WORK_CONTEXT; +// Forward declarations +// #pragma code_seg("PAGE") _IRQL_requires_max_(PASSIVE_LEVEL) static void DMF_BusFilter_Relations_RemoveDevice( - _In_ PDEVICE_OBJECT DeviceObject + _In_ PDEVICE_OBJECT DeviceObject, + _In_ PIRP RemoveIrp ); #pragma code_seg() @@ -170,7 +199,8 @@ _IRQL_requires_max_(PASSIVE_LEVEL) static void DMF_BusFilter_Relations_RemoveDevice( - _In_ PDEVICE_OBJECT DeviceObject + _In_ PDEVICE_OBJECT DeviceObject, + _In_ PIRP RemoveIrp ) /*++ @@ -179,9 +209,22 @@ Routine Description: Processes child device removal. Always performs a full teardown regardless of the IsExisting flag so that surprise removals are handled correctly. + Mandatory teardown order (WDM filter driver guidance): + 1. IoReleaseRemoveLockAndWait -- drain all in-flight I/O holders; after this + returns, no new I/O can enter the device. + 2. RemoveEntryList -- unlink from the parent's ChildList. + 3. EvtDeviceRemove callback -- client notification. + 4. WdfObjectDelete -- destroy the WDF wrapper object. + 5. IoDetachDevice -- detach filter from the PDO stack. + 6. IoDeleteDevice -- lazy-free the filter device object (memory + released when the last reference drops; do + NOT touch DeviceExtension after this call). + Arguments: DeviceObject - Filter device object whose stack is being torn down. + RemoveIrp - The IRP_MN_REMOVE_DEVICE IRP; used as the remove-lock tag to + match the IoAcquireRemoveLock call made in DispatchHandler. Return Value: @@ -199,9 +242,17 @@ Return Value: PAGED_CODE(); - // Always remove from list and tear down on REMOVE_DEVICE, regardless of - // IsExisting state. A surprise removal can arrive while IsExisting is TRUE, - // and skipping teardown leaves the filter attached to a dead stack. + // Step 1: release the remove lock (matching the IoAcquireRemoveLock in + // DispatchHandler) AND wait for all other in-flight holders to drain. + // After this call, no concurrent dispatcher can be accessing the extension + // and no new dispatch will succeed (IoAcquireRemoveLock returns + // STATUS_DELETE_PENDING). + // + IoReleaseRemoveLockAndWait(&extension->RemoveLock, RemoveIrp); + + TraceVerbose(DMF_TRACE, "%!FUNC! called at %!irql!", KeGetCurrentIrql()); + + // Step 2: unlink from parent's list while holding the spinlock. // #pragma warning(disable: 28150) // elevates to DISPATCH_LEVEL KeAcquireInStackQueuedSpinLock(&parentContext->ChildListLock, @@ -210,8 +261,8 @@ Return Value: KeReleaseInStackQueuedSpinLock(&handle); #pragma warning(default: 28150) // drops to PASSIVE_LEVEL - TraceVerbose(DMF_TRACE, "%!FUNC! called at %!irql!", KeGetCurrentIrql()); - + // Steps 3-6: notify client, then destroy objects in the correct order. + // if (config->EvtDeviceRemove) { config->EvtDeviceRemove(extension->Parent, extension->Child); @@ -219,7 +270,7 @@ Return Value: WdfObjectDelete(extension->Child); IoDetachDevice(extension->TargetDeviceObject); - IoDeleteDevice(DeviceObject); + IoDeleteDevice(DeviceObject); // Extension is invalid after this point FuncExitNoReturn(DMF_TRACE); } @@ -244,6 +295,10 @@ Routine Description: Handles PnP requests for child filter devices. + For IRP_MN_REMOVE_DEVICE the remove lock is released inside + Relations_RemoveDevice via IoReleaseRemoveLockAndWait. Callers must NOT + call IoReleaseRemoveLock again after this function returns for that minor code. + Arguments: DeviceObject - Child filter device object. @@ -282,8 +337,10 @@ Return Value: IoCompleteRequest(Irp, IO_NO_INCREMENT); // Tear down now that the IRP has been forwarded and completed. + // Relations_RemoveDevice calls IoReleaseRemoveLockAndWait, which + // releases the lock that was acquired in DispatchHandler for this IRP. // - DMF_BusFilter_Relations_RemoveDevice(DeviceObject); + DMF_BusFilter_Relations_RemoveDevice(DeviceObject, Irp); return ntStatus; } @@ -314,12 +371,28 @@ DMF_BusFilter_DispatchHandler( Routine Description: - Dispatch routine handler for all IRPs + Dispatch routine handler for all IRPs. + + For child devices (those tagged with GUID_DMF_BUSFILTER_SIGNATURE), the + remove lock is acquired here to serialize all dispatch activity against + IRP_MN_REMOVE_DEVICE teardown. + + Remove-lock release: + - IRP_MN_REMOVE_DEVICE: released inside Relations_RemoveDevice via + IoReleaseRemoveLockAndWait (do NOT release again here). + - All other PnP minors: released after DispatchPnp returns. + - Non-PnP IRPs: released BEFORE IoCallDriver (after saving TargetDeviceObject + to a local) so the lock does not need to span an asynchronous IRP completion. + + Power IRP passthrough: IRP_MJ_POWER is forwarded with IoSkip + IoCallDriver + without calling PoStartNextPowerIrp. This is correct on the Windows 10+ + baseline targeted by this library (PoStartNextPowerIrp is obsolete; the I/O + manager handles power IRP serialisation automatically). Arguments: - DeviceObject - Parent device object. - Irp - Irp with request. + DeviceObject - Target device object. + Irp - Irp with request. Return Value: @@ -327,27 +400,59 @@ Return Value: --*/ { - const WDM_CHILD_DEVICE_EXTENSION* extension = (WDM_CHILD_DEVICE_EXTENSION*)DeviceObject->DeviceExtension; + NTSTATUS ntStatus; + WDM_CHILD_DEVICE_EXTENSION* extension = (WDM_CHILD_DEVICE_EXTENSION*)DeviceObject->DeviceExtension; const PIO_STACK_LOCATION stack = IoGetCurrentIrpStackLocation(Irp); const BusFilter_Context* context = BusFilterContextGet(WdfGetDriver()); + // If the device object does not have our signature it is the WDF FDO (or + // another device object owned by this driver). Delegate to the saved handler. + // if (!IsEqualGUID(&extension->Signature, &GUID_DMF_BUSFILTER_SIGNATURE)) { return context->MajorDispatchFunctions[stack->MajorFunction](DeviceObject, Irp); } - // Handle PNP requests - // - if (stack->MajorFunction == IRP_MJ_PNP) + // Acquire the remove lock before touching the extension or forwarding the IRP. + // This prevents IRP_MN_REMOVE_DEVICE teardown from deleting the device object + // while we are still inside the dispatch routine. + // + ntStatus = IoAcquireRemoveLock(&extension->RemoveLock, Irp); + if (!NT_SUCCESS(ntStatus)) { - return DMF_BusFilter_DispatchPnp(DeviceObject, Irp, stack->MinorFunction); + // Teardown is in progress; reject new I/O. + Irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(Irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; } - // Forward to lower driver - // + if (stack->MajorFunction == IRP_MJ_PNP) + { + ntStatus = DMF_BusFilter_DispatchPnp(DeviceObject, Irp, stack->MinorFunction); + + // For REMOVE_DEVICE, IoReleaseRemoveLockAndWait inside Relations_RemoveDevice + // has already released the lock (and waited for all other holders). A second + // release here would corrupt the lock count. + // + if (stack->MinorFunction != IRP_MN_REMOVE_DEVICE) + { + IoReleaseRemoveLock(&extension->RemoveLock, Irp); + } + + return ntStatus; + } + + // Non-PnP pass-through: save TargetDeviceObject then release the lock before + // IoCallDriver so the remove-lock does not span the asynchronous completion. + // After the release, IoDeleteDevice may run concurrently but IoCallDriver does + // not reference the filter extension -- only the already-saved target pointer. + // + PDEVICE_OBJECT targetDevice = extension->TargetDeviceObject; + IoReleaseRemoveLock(&extension->RemoveLock, Irp); + IoSkipCurrentIrpStackLocation(Irp); - return IoCallDriver(extension->TargetDeviceObject, Irp); + return IoCallDriver(targetDevice, Irp); } /////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -355,7 +460,7 @@ Return Value: /////////////////////////////////////////////////////////////////////////////////////////////////////// // -// IoCreateDevice and IoAttachDeviceToDeviceStack both require PASSIVE_LEVEL. +// IoCreateDevice and IoAttachDeviceToDeviceStackSafe both require PASSIVE_LEVEL. // The annotation is corrected from the original APC_LEVEL. // Note: this function is NOT in the PAGE segment because it acquires a spinlock // (which momentarily elevates to DISPATCH_LEVEL), making paged memory unsafe. @@ -371,7 +476,19 @@ DMF_BusFilter_Relations_AddDevice( Routine Description: - Creates proxy child device for bus PDO. + Creates a proxy filter device object for a bus PDO. + + Attach window: bus-relations completion time (i.e. after the lower bus + driver has populated DEVICE_RELATIONS but before the PnP manager has + dispatched IRP_MN_START_DEVICE to the child devnode's FDO) is the only + safe window for a bus filter to attach. Attaching earlier races the PDO + creation; attaching later races FDO construction and IRP dispatch. + + STATUS_MORE_PROCESSING_REQUIRED in the completion routine (which calls this + function) keeps the DEVICE_RELATIONS array -- and therefore the bus driver's + references on each PDO -- alive until we finish attaching. Without that hold, + the PDO could be released and deleted while IoAttachDeviceToDeviceStackSafe + is executing. Arguments: @@ -410,7 +527,7 @@ Return Value: &handle); // Find and update PDO status - // + // for ( entry = parentContext->ChildList.Flink; entry != &parentContext->ChildList; @@ -442,7 +559,7 @@ Return Value: attributes.ParentObject = Device; // Create piggyback framework object for WDM child device object - // + // ntStatus = WdfObjectCreate(&attributes, (WDFOBJECT*)&child); if (!NT_SUCCESS(ntStatus)) { @@ -450,12 +567,20 @@ Return Value: goto Exit; } + // Derive the device type from the PDO when the client did not specify one. + // A filter whose device type differs from the stack it joins can cause + // I/O manager policy mismatches (e.g. buffering mode checks). + // + const DEVICE_TYPE deviceType = (config->DeviceType != 0) + ? config->DeviceType + : PhysicalDeviceObject->DeviceType; + // Create WDM device - // + // ntStatus = IoCreateDevice(deviceObject->DriverObject, sizeof(WDM_CHILD_DEVICE_EXTENSION), NULL, - config->DeviceType, + deviceType, FILE_DEVICE_SECURE_OPEN | config->DeviceCharacteristics, FALSE, &filterDeviceObject); @@ -466,7 +591,7 @@ Return Value: } // Link WDM and WDF device together. - // + // childContext = DMF_BusFilter_GetChildContext(child); childContext->DeviceObject = filterDeviceObject; @@ -476,18 +601,33 @@ Return Value: RtlCopyMemory(&childExtension->Signature, &GUID_DMF_BUSFILTER_SIGNATURE, sizeof(GUID)); + + // Initialise the remove lock before the device is attached to any stack + // or its DO_DEVICE_INITIALIZING flag is cleared. The lock must be ready + // before the first possible IoAcquireRemoveLock call. + // + IoInitializeRemoveLock(&childExtension->RemoveLock, + DMF_BUSFILTER_POOL_TAG, + 0, // MaxLockedMinutes (0 = no timeout in free builds) + 0); // HighWatermark (0 = no per-lock tag tracking) + childExtension->Parent = Device; childExtension->Child = child; - childExtension->PhysicalDeviceObject = PhysicalDeviceObject; - childExtension->TargetDeviceObject = IoAttachDeviceToDeviceStack(filterDeviceObject, - PhysicalDeviceObject); - if (childExtension->TargetDeviceObject == NULL) + // Use the safe variant to avoid a stale-pointer race: IoAttachDeviceToDeviceStack + // (non-safe) can return a pointer to a device that is concurrently being deleted. + // IoAttachDeviceToDeviceStackSafe returns an NTSTATUS and a valid target pointer + // only if the target is still alive when the attach completes. + // + ntStatus = IoAttachDeviceToDeviceStackSafe(filterDeviceObject, + PhysicalDeviceObject, + &childExtension->TargetDeviceObject); + if (!NT_SUCCESS(ntStatus)) { + TraceError(DMF_TRACE, "IoAttachDeviceToDeviceStackSafe fails: ntStatus=%!STATUS!", ntStatus); IoDeleteDevice(filterDeviceObject); filterDeviceObject = NULL; - ntStatus = STATUS_NO_SUCH_DEVICE; goto Exit; } @@ -513,7 +653,7 @@ Return Value: &handle); // Re-scan under the lock before inserting to guard against a duplicate created - // by a concurrent bus-relations completion (Finding 5). + // by a concurrent bus-relations completion. // BOOLEAN duplicate = FALSE; for ( @@ -554,6 +694,9 @@ Return Value: goto Exit; } + // The device is fully initialised; clear the initialising flag to allow + // IRPs to be dispatched to it. + // filterDeviceObject->Flags &= ~DO_DEVICE_INITIALIZING; ntStatus = STATUS_SUCCESS; @@ -642,8 +785,8 @@ DMF_BusFilter_ProcessBusRelations( #pragma code_seg() // Work item routine: runs at PASSIVE_LEVEL when the completion routine fires at -// DISPATCH_LEVEL and cannot call IoCreateDevice / IoAttachDeviceToDeviceStack -// directly (Finding 3). +// DISPATCH_LEVEL and cannot call IoCreateDevice / IoAttachDeviceToDeviceStackSafe +// directly. // static IO_WORKITEM_ROUTINE DMF_BusFilter_QueryBusRelationsWorker; @@ -665,8 +808,21 @@ DMF_BusFilter_QueryBusRelationsWorker( } PIRP irp = workCtx->Irp; + WDFDEVICE device = workCtx->Device; - DMF_BusFilter_ProcessBusRelations(workCtx->Device, irp); + DMF_BusFilter_ProcessBusRelations(device, irp); + + // Signal the idle event if this was the last outstanding work item, so that + // DeviceCleanup can proceed once all workers have finished. + // + PARENT_BUS_DEVICE_CONTEXT* parentContext = DMF_BusFilter_GetParentContext(device); + if (parentContext != NULL) + { + if (InterlockedDecrement(&parentContext->OutstandingWorkItems) == 0) + { + KeSetEvent(&parentContext->WorkItemsIdle, IO_NO_INCREMENT, FALSE); + } + } IoFreeWorkItem(workCtx->WorkItem); ExFreePoolWithTag(workCtx, DMF_BUSFILTER_POOL_TAG); @@ -683,6 +839,12 @@ DMF_BusFilter_QueryBusRelationsWorker( // PAGED_CODE() because IO completion routines may run at DISPATCH_LEVEL. // When that happens the passive-level AddDevice work is deferred to a work item. // +// Returning STATUS_MORE_PROCESSING_REQUIRED to hold the IRP is intentional and +// load-bearing: it keeps the DEVICE_RELATIONS array (and the bus driver's +// references on each PDO) alive until the passive-level worker has finished +// attaching filter devices. Without this hold the PDOs could be released before +// IoAttachDeviceToDeviceStackSafe completes. +// _IRQL_requires_max_(DISPATCH_LEVEL) static NTSTATUS @@ -724,7 +886,7 @@ Return Value: } // If completion is running at DISPATCH_LEVEL, IoCreateDevice and - // IoAttachDeviceToDeviceStack cannot be called safely. Defer to a + // IoAttachDeviceToDeviceStackSafe cannot be called safely. Defer to a // passive-level work item. // if (KeGetCurrentIrql() > PASSIVE_LEVEL) @@ -735,12 +897,23 @@ Return Value: DMF_BUSFILTER_POOL_TAG); if (workCtx != NULL) { - workCtx->Device = Device; - workCtx->Irp = Irp; + workCtx->Device = Device; + workCtx->Irp = Irp; workCtx->WorkItem = IoAllocateWorkItem(WdfDeviceWdmGetDeviceObject(Device)); if (workCtx->WorkItem != NULL) { + // Increment the outstanding-work counter BEFORE queuing. + // The worker decrements it and signals the idle event when done. + // This prevents DeviceCleanup from racing the worker. + // + PARENT_BUS_DEVICE_CONTEXT* parentContext = DMF_BusFilter_GetParentContext(Device); + if (parentContext != NULL) + { + InterlockedIncrement(&parentContext->OutstandingWorkItems); + KeClearEvent(&parentContext->WorkItemsIdle); + } + IoQueueWorkItem(workCtx->WorkItem, DMF_BusFilter_QueryBusRelationsWorker, DelayedWorkQueue, @@ -783,8 +956,8 @@ Routine Description: Arguments: - DeviceObject - Parent device object. - Irp - IRP_MJ_PNP / IRP_MN_QUERY_DEVICE_RELATIONS request. + Device - Parent WDF device. + Irp - IRP_MJ_PNP / IRP_MN_QUERY_DEVICE_RELATIONS request. Return Value: @@ -952,7 +1125,7 @@ Return Value: FuncEntry(DMF_TRACE); // Forward immediately if client driver has no handler - // + // if (config->EvtDeviceQueryId == NULL) { IoSkipCurrentIrpStackLocation(Irp); @@ -964,11 +1137,11 @@ Return Value: } // If client driver didn't do anything with the IRP... - // + // if (!config->EvtDeviceQueryId(ChildDevice, Irp)) { // ...forward it prior to completion - // + // if (!IoForwardIrpSynchronously(DMF_BusFilter_WdmAttachedDeviceGet(ChildDevice), Irp)) { @@ -1018,7 +1191,7 @@ Return Value: FuncEntry(DMF_TRACE); // Forward immediately if client driver has no handler - // + // if (config->EvtDeviceQueryInterface == NULL) { IoSkipCurrentIrpStackLocation(Irp); @@ -1030,11 +1203,11 @@ Return Value: } // If client driver didn't do anything with the IRP... - // + // if (!config->EvtDeviceQueryInterface(ChildDevice, Irp)) { // ...forward it prior to completion - // + // if (!IoForwardIrpSynchronously(DMF_BusFilter_WdmAttachedDeviceGet(ChildDevice), Irp)) { @@ -1093,7 +1266,7 @@ Return Value: contextBusFilter = NULL; // Config is required - // + // if (BusFilterConfig == NULL) { ntStatus = STATUS_INVALID_PARAMETER; @@ -1101,7 +1274,7 @@ Return Value: } // Driver object must be already created - // + // if (!WdfGetDriver()) { ntStatus = STATUS_NOT_SUPPORTED; @@ -1118,18 +1291,18 @@ Return Value: BusFilter_Context); // Attach context to driver object. - // + // ntStatus = WdfObjectAllocateContext(WdfGetDriver(), &attributes, (void**)&contextBusFilter); if (!NT_SUCCESS(ntStatus)) { - TraceError(DMF_TRACE, "WdfObjectAllocateContext fails: ntStatus=%!STATUS!",ntStatus); + TraceError(DMF_TRACE, "WdfObjectAllocateContext fails: ntStatus=%!STATUS!", ntStatus); goto Exit; } // Save copy of config in context to invoke callback routines later. - // + // RtlCopyMemory(&contextBusFilter->Configuration, BusFilterConfig, sizeof(DMF_BusFilter_CONFIG)); @@ -1138,7 +1311,7 @@ Return Value: PDRIVER_DISPATCH* pDispatch; // Store original dispatch routine pointers and overwrite with our own - // + // #pragma warning(disable:28175) for ( index = 0, @@ -1153,14 +1326,14 @@ Return Value: #pragma warning(default:28175) // PnP minor code dispatch routines - // + // contextBusFilter->PnPMinorDispatchFunctions[IRP_MN_START_DEVICE] = DMF_BusFilter_PnP_StartDevice; contextBusFilter->PnPMinorDispatchFunctions[IRP_MN_DEVICE_ENUMERATED] = DMF_BusFilter_PnP_DeviceEnumerated; contextBusFilter->PnPMinorDispatchFunctions[IRP_MN_QUERY_ID] = DMF_BusFilter_PnP_QueryId; contextBusFilter->PnPMinorDispatchFunctions[IRP_MN_QUERY_INTERFACE] = DMF_BusFilter_PnP_QueryInterface; // Clear invalid characteristics (see MS docs) - // + // BusFilterConfig->DeviceCharacteristics &= ~(FILE_AUTOGENERATED_DEVICE_NAME | FILE_CHARACTERISTIC_TS_DEVICE | FILE_CHARACTERISTIC_WEBDAV_DEVICE | @@ -1188,11 +1361,11 @@ DMF_BusFilter_DeviceAdd( Routine Description: - Creates bus WDF device. + Creates bus WDF device. Arguments: - Driver - Associated WDFDRIVER. + Driver - Associated WDFDRIVER. DeviceInit - WDF PWDFDEVICE_INIT. Return Value: @@ -1226,7 +1399,7 @@ Return Value: WdfFdoInitSetFilter(DeviceInit); // Attach IRP preprocessor. - // + // ntStatus = WdfDeviceInitAssignWdmIrpPreprocessCallback(DeviceInit, DMF_BusFilter_PreprocessQueryBusRelations, IRP_MJ_PNP, @@ -1240,11 +1413,11 @@ Return Value: // Don't initialize with context here as client driver might decide // to set their own context memory in EvtPreBusDeviceAdd - // + // WDF_OBJECT_ATTRIBUTES_INIT(&attributes); // Call pre-device-creation callback, if set. - // + // if (config->EvtPreBusDeviceAdd) { ntStatus = config->EvtPreBusDeviceAdd(Driver, @@ -1259,14 +1432,14 @@ Return Value: } // Client driver is using DMF modules. - // + // if (dmfDeviceInit != NULL) { DMF_DmfFdoSetFilter(dmfDeviceInit); } // Create device object - // + // ntStatus = WdfDeviceCreate(&DeviceInit, &attributes, &device); @@ -1281,7 +1454,7 @@ Return Value: PARENT_BUS_DEVICE_CONTEXT); // Add bus device context. - // + // ntStatus = WdfObjectAllocateContext(device, &attributes, (void**)&parentContext); @@ -1294,8 +1467,14 @@ Return Value: InitializeListHead(&parentContext->ChildList); KeInitializeSpinLock(&parentContext->ChildListLock); + // Initialise work-item rundown state. OutstandingWorkItems starts at zero + // and WorkItemsIdle starts signaled (no workers pending). + // + parentContext->OutstandingWorkItems = 0; + KeInitializeEvent(&parentContext->WorkItemsIdle, NotificationEvent, TRUE); + // Call post-device-creation callback, if set. - // + // if (config->EvtPostBusDeviceAdd) { ntStatus = config->EvtPostBusDeviceAdd(device, @@ -1438,10 +1617,22 @@ DMF_BusFilter_DeviceCleanup( Routine Description: Tears down all remaining child filter devices associated with a bus device. - The client driver should call this from its EvtDeviceReleaseHardware or - EvtCleanupCallback to ensure WDM filter devices are not leaked on unload. - Without this call, any child devices still in ChildList at unload time are - leaked and will fault when the I/O manager later dereferences them. + + This function must be called from the client driver's EvtDeviceReleaseHardware + or EvtCleanupCallback AFTER PnP has already dispatched IRP_MN_REMOVE_DEVICE to + all child devnodes. Under normal operation the ChildList is empty by the time + this function runs (each REMOVE_DEVICE already drained it). This function acts + as a safety net for abnormal paths (e.g. parent surprise-removed before all + children received REMOVE). + + For each remaining child: + - Acquires the per-child remove lock (prevents new I/O from entering). + - Calls IoReleaseRemoveLockAndWait to drain any in-flight I/O. + - Detaches and deletes the filter device in the mandatory order. + + If IoAcquireRemoveLock returns STATUS_DELETE_PENDING for a child, it means an + IRP_MN_REMOVE_DEVICE is concurrently processing that child; this function logs + the anomaly and skips it (the REMOVE path owns teardown in that case). Arguments: @@ -1466,27 +1657,71 @@ Return Value: goto Exit; } - // Drain ChildList, detaching and deleting each filter device. - // The lock is released around the passive-level teardown calls because - // IoDetachDevice and IoDeleteDevice cannot be called while holding a spinlock. + // Wait for any bus-relations work items that are still executing to finish + // before we start tearing down ChildList and its entries. Without this wait, + // a passive-level worker could be calling Relations_AddDevice (touching the + // list and child extensions) while we are freeing them. + // + KeWaitForSingleObject(&parentContext->WorkItemsIdle, + Executive, + KernelMode, + FALSE, + NULL); + + // Drain ChildList. The lock is released around per-child teardown because + // IoReleaseRemoveLockAndWait, IoDetachDevice, and IoDeleteDevice cannot be + // called while holding a spinlock. // #pragma warning(disable: 28150) KeAcquireInStackQueuedSpinLock(&parentContext->ChildListLock, &handle); while (!IsListEmpty(&parentContext->ChildList)) { - LIST_ENTRY* entry = RemoveHeadList(&parentContext->ChildList); - - KeReleaseInStackQueuedSpinLock(&handle); - + // Peek at the head of the list without removing it until we know + // we can safely take ownership of this child. + // + LIST_ENTRY* entry = parentContext->ChildList.Flink; WDM_CHILD_DEVICE_EXTENSION* childExtension = CONTAINING_RECORD(entry, WDM_CHILD_DEVICE_EXTENSION, ListEntry); - PDEVICE_OBJECT filterDevice = DMF_BusFilter_WdmDeviceObjectGet(childExtension->Child); - PDEVICE_OBJECT targetDevice = childExtension->TargetDeviceObject; - DMFBUSCHILDDEVICE child = childExtension->Child; - WDFDEVICE parent = childExtension->Parent; + PDEVICE_OBJECT filterDevice = DMF_BusFilter_WdmDeviceObjectGet(childExtension->Child); + // Try to acquire the remove lock while still holding the spinlock. + // IoAcquireRemoveLock is callable at DISPATCH_LEVEL. + // + NTSTATUS lockStatus = IoAcquireRemoveLock(&childExtension->RemoveLock, filterDevice); + if (!NT_SUCCESS(lockStatus)) + { + // An IRP_MN_REMOVE_DEVICE is already processing this child. That path + // will call RemoveEntryList under the spinlock, so we must not touch + // this entry. Remove it from the list now to avoid looping forever, + // and let the REMOVE path complete teardown on its own. + // + TraceWarning(DMF_TRACE, "%!FUNC! child device already removing (concurrent REMOVE_DEVICE)"); + RemoveEntryList(entry); + continue; // re-check IsListEmpty with lock still held + } + + // We successfully acquired the lock; take the entry out of the list. + // + RemoveEntryList(entry); + KeReleaseInStackQueuedSpinLock(&handle); + + // Save locals before any pointer might become invalid. + // + PDEVICE_OBJECT targetDevice = childExtension->TargetDeviceObject; + DMFBUSCHILDDEVICE child = childExtension->Child; + WDFDEVICE parent = childExtension->Parent; + + // Drain in-flight I/O: IoReleaseRemoveLockAndWait marks the lock + // "removing" (so new IoAcquireRemoveLock calls fail), releases the + // acquire we just made AND the implicit initial reference, then waits + // for the count to reach zero. + // + IoReleaseRemoveLockAndWait(&childExtension->RemoveLock, filterDevice); + + // Notify client, then tear down in mandatory order. + // if (context->Configuration.EvtDeviceRemove) { context->Configuration.EvtDeviceRemove(parent, child); @@ -1497,7 +1732,7 @@ Return Value: if (filterDevice != NULL) { - IoDeleteDevice(filterDevice); + IoDeleteDevice(filterDevice); // Extension invalid after this } KeAcquireInStackQueuedSpinLock(&parentContext->ChildListLock, &handle);