[PATCH 4/6] ntoskrnl.exe: Send IRP_MN_SURPRISE_REMOVAL before removing children.

[Zebediah Figura (she/her)]

On 6/18/21 1:49 PM, Rémi Bernon wrote:
> On 6/18/21 5:59 PM, Zebediah Figura (she/her) wrote:
>> On 6/18/21 7:06 AM, Rémi Bernon wrote:
>>> So that mini driver gets notified first and has a chance to cancel
>>> pending IRPs.
>>
>> Which minidriver?
>>
>> Isn't it the responsibility of the child to terminate all pending IRPs
>> (and disallow further ones) on removal? See also [0], [1], [2], which
>> say that queued requests should be completed in (both, for some reason)
>> IRP_MN_SURPRISE_REMOVAL and IRP_MN_REMOVE_DEVICE.
>>
>> I think this deserves tests.
>>
>> [0]
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/removing-a-device-in-a-function-driver
>>
>>
>> [1]
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/handling-an-irp-mn-surprise-removal-request
>>
>>
>> [2]
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/removing-a-device-in-a-bus-drive
>>
>>
> 
> I really don't have the big picture yet, so I meant the one implemented
> in driver_hid.c. If it doesn't cancel the IRP it has queued on device
> removal, the test never completes the device removal on Windows.
> 
> On Wine, driver_hid never receives the IRP_MN_SURPRISE_REMOVAL if we
> don't call it before removing the "children" in ntoskrnl.exe. I don't
> really understand who are the children there, but driver_hid apparently
> isn't.
> 

Allow me to explain in detail. Apologies if any of this is old news to 
you. If nothing else I'll adapt it to a wiki page later ;-)


TLC PDO
                      winehid.sys + hidclass.sys
HID FDO
-------------------------------------------------
HID PDO
                      winebus.sys
root FDO
-------------------------------------------------
root PDO
                      ntoskrnl.exe
[PNP manager]


In the case of the HID test driver it's a little different:


TLC PDO
                      driver_hid.dll + hidclass.sys
root/HID FDO
-------------------------------------------------
root/HID PDO
                      ntoskrnl.exe
[PNP manager]


A root PNP device is the one passed to AddDevice for a root PNP driver. 
Usually it's intended to enumerate children; here we kind of cheat and 
make the root PnP device the same device as the HID device. It's a bit 
weird but it works.

The TLC (top-level collection) PDO is managed entirely within 
hidclass.sys; the minidriver never interacts with it. In theory there's 
supposed to be one or more of these; we don't handle the "or more" case yet.

The application only ever interacts with the TLC PDO. It's actually 
impossible to open the HID device stack (IIRC it fails with 
STATUS_UNSUCCESSFUL). In response to read/write/hidd ioctls, hidclass 
creates new irps and sends them to the HID device stack, i.e. the "HID 
FDO/HID PDO" pair in the above diagrams.

[Note that IRPs are always sent to the top of a stack first, so in this 
case the minidriver has a chance to handle it. In a real HID driver the 
minidriver would always handle it itself, rather than sending it down to 
the PDO. You can mentally replace winehid with hidusb, and winebus with 
(say) usbccgp. Obviously usbccgp doesn't know anything about HID. Our 
architecture is a little weird, though, for various reasons.]

There is usually no TLC FDO, but in theory there could be, and in 
practice I believe mouse and keyboard drivers attach to the TLC PDO.

On reflection, I think I may have confused myself, so this patch 
actually makes more sense than it initially did. It'd still be nice to 
have tests, though, and the commit message should ideally be more specific.