From cea19d2e0f62d69d57576299b2fafc2a3ee9b2ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Fri, 27 Dec 2024 13:44:15 +0100 Subject: [PATCH] [NTOS:IO] IopAttachDeviceToDeviceStackSafe(): Attach the device to the device list UNDER lock! This function is the internal helper for the `IoAttachDevice*()` functions, in particular for `IoAttachDeviceToDeviceStackSafe()`. Because the function modifies the chained list of stacked devices, it must hold the device list lock (the I/O system database lock) while doing the devices attachment. In particular, modifying the `SourceDevice`'s extension `AttachedTo` field, but also modifying its other fields and the `AttachedDevice` ones as well. This fix is similar to the one already committed in f8cbc3e48c (r70496). ---- In MSDN [^1] it is indicated (although not as clearly as it could be) that `IoAttachDeviceToDeviceStackSafe()` sets the returned `AttachedToDeviceObject` pointer under the device list lock. The reason is best spelled in [^2][^3]. Indeed, when a filter attaches to a lower PDO (`PhysicalDevice`) by doing: ```c myDeviceExtension->LowerDevice = IoAttachDeviceToDeviceStack(myFilterDevice, PhysicalDevice); ``` there exists a time window where the function finished attaching the filter device to the PDO, but hasn't yet returned the device at the top of the stack to be stored in `myDeviceExtension->LowerDevice` (which gets used later internally by the filter to pass IRPs down the device stack). During this time, the filter device may receive some IRPs and its dispatch routine would use a not-yet initialized `LowerDevice` member. The IoAttachDeviceToDeviceStackSafe() allows doing: ```c Status = IoAttachDeviceToDeviceStackSafe( myFilterDevice, PhysicalDevice, &myDeviceExtension->LowerDevice); ``` and forbidding the IRPs to be delivered to the filter device, while the `LowerDevice` member is being initialized with the device list lock held. ---- [^1]: "IoAttachDeviceToDeviceStackSafe function (ntddk.h)" https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/nf-ntddk-ioattachdevicetodevicestacksafe#remarks [^2]: "IoAttachDeviceToDeviceStack gotcha" (Satya Das, Winprogger) https://winprogger.com/ioattachdevicetodevicestack-gotcha/ [^3]: Community OSR answer (by Tony Mason) https://community.osr.com/t/attach-filter-driver/9450/3 --- ntoskrnl/io/iomgr/device.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ntoskrnl/io/iomgr/device.c b/ntoskrnl/io/iomgr/device.c index 17941283741..d56e6f6881e 100644 --- a/ntoskrnl/io/iomgr/device.c +++ b/ntoskrnl/io/iomgr/device.c @@ -73,6 +73,10 @@ IopAttachDeviceToDeviceStackSafe( { PDEVICE_OBJECT AttachedDevice; PEXTENDED_DEVOBJ_EXTENSION SourceDeviceExtension; + KIRQL OldIrql; + + /* Lock the device list while attaching the device */ + OldIrql = KeAcquireQueuedSpinLock(LockQueueIoDatabaseLock); /* Get the attached device and source extension */ AttachedDevice = IoGetAttachedDevice(TargetDevice); @@ -107,9 +111,13 @@ IopAttachDeviceToDeviceStackSafe( SourceDeviceExtension->AttachedTo = AttachedDevice; } - /* Return the attached device */ + /* Return the attached device (under the lock) */ if (AttachedToDeviceObject) *AttachedToDeviceObject = AttachedDevice; + + /* Release the device list lock */ + KeReleaseQueuedSpinLock(LockQueueIoDatabaseLock, OldIrql); + return AttachedDevice; }