Pdo_DevicePropertyTableWrite adds properties to parent device instead of PDO

[microsoft]

Pdo_DevicePropertyTableWrite adds properties to parent device instead of PDO

[samtertzakian]

Release v. 1.1.122 has the fix for the originally reported issue.

[nefarius]

I started work on a fix in parallel to opening this issue but I am afraid I am stuck and can't quite figure out why; when I invoke WdfDeviceAssignProperty with the child device handle, I consistently get this error:

[Pdo_PdoEx]Pdo_DevicePropertyTableWrite fails: ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)

Here's my diff, I can not figure out the mistake 😕

diff --git "a/Dmf/Modules.Library/Dmf_Pdo.c" "b/Dmf/Modules.Library/Dmf_Pdo.c"
index 29014f5..5159254 100644
--- "a/Dmf/Modules.Library/Dmf_Pdo.c"
+++ "b/Dmf/Modules.Library/Dmf_Pdo.c"
@@ -78,6 +78,7 @@ _Must_inspect_result_
 NTSTATUS
 Pdo_DevicePropertyTableWrite(
     _In_ DMFMODULE DmfModule,
+    _In_ WDFDEVICE ChildDevice,
     _In_ Pdo_DeviceProperty_Table* DevicePropertyTable
     )
 /*++
@@ -99,7 +100,6 @@ Return Value:
 --*/
 {
     NTSTATUS ntStatus;
-    WDFDEVICE device;
     Pdo_DevicePropertyEntry* entry;
     UNREFERENCED_PARAMETER(DmfModule);
 
@@ -112,7 +112,6 @@ Return Value:
     // Assign the properties for this device.
     //
     ntStatus = STATUS_SUCCESS;
-    device = DMF_ParentDeviceGet(DmfModule);
     for (ULONG propertyIndex = 0; propertyIndex < DevicePropertyTable->ItemCount; propertyIndex++)
     {
         entry = &DevicePropertyTable->TableEntries[propertyIndex];
@@ -131,7 +130,7 @@ Return Value:
                 goto Exit;
             }
 
-            ntStatus = WdfDeviceCreateDeviceInterface(device, 
+            ntStatus = WdfDeviceCreateDeviceInterface(ChildDevice, 
                                                       entry->DeviceInterfaceGuid,
                                                       NULL);
             if (!NT_SUCCESS(ntStatus))
@@ -140,10 +139,10 @@ Return Value:
                 goto Exit;
             }
         }
-
+        
         // Now set the properties.
         //
-        ntStatus = WdfDeviceAssignProperty(device,
+        ntStatus = WdfDeviceAssignProperty(ChildDevice,
                                            &entry->DevicePropertyData,
                                            entry->ValueType,
                                            entry->ValueSize,
@@ -503,7 +502,8 @@ Return Value:
     //
     if (PdoRecord->DeviceProperties != NULL)
     {
-        ntStatus = Pdo_DevicePropertyTableWrite(DmfModule, 
+        ntStatus = Pdo_DevicePropertyTableWrite(DmfModule,
+												childDevice,
                                                 PdoRecord->DeviceProperties);
         if (!NT_SUCCESS(ntStatus))
         {

[samtertzakian]

Let me investigate on my end and get back to you.

[samtertzakian]

Update:

We are actively investigating. We have determined that WDF specifically prevents that call (WdfDeviceAssignProperty()) against a Child WDFDEVICE after creation. But we are investigating further to try to understand why that is the case. I will update this thread when I have more information.

[samtertzakian]

Ok I have corrected the bug and updated the test to verify more than one property is written. Let me go through review/verification. I will try to update the branch later today. After we run the regression tests and you verify it works for you we will merge the branch.

Thank you for your patience and your help.

[samtertzakian]

OK, I believe I have reproduced the device interface issue you see.

From what I can see, the device interface is successfully created. But then, in IoctlHandler_PostDeviceInterfaceCreate() the call to WdfDeviceRetrieveDeviceInterfaceString() fails with STATUS_INVALID_DEVICE_STATE.

It calls that code to set properties which it cannot do in this case.

Let me discuss all of this with others here so we can determine what our options are.

For the time being, it looks to me like you can temporarily disable the call to IoctlHandler_PostDeviceInterfaceCreate() in the function IoctlHandler_DeviceInterfaceCreate(). I just tried it and it seems to work to me.

[nefarius]

I think for the time being the IOCTL handler is operable despite the error so no haste, I see my new callbacks invoked so I can continue my migration adventure.

[samtertzakian]

Release v1.1.123 corrects the second reported issue in this thread. So, this thread can be closed.

[samtertzakian]

Yes, it appears you are correct. It should be on the Child Device...same for line 134 just above in the call to create the device interface. For now, can you create a PR? Meanwhile, I will verify on here to make sure. Thank you for your feedback!

[samtertzakian]

Ok, yes...We have verified your assertion.

We need to update 2 lines: DMF_Pdo.c#134,146. Also, we probably should make it possible to set the Device Interface Reference String on 146 (separate issue). If you don't want to do that, we can do it here.

Please make a PR. Or if you don't have time, we will do it and have in the next merge.

Thank you!

[nefarius]

Update:

We are actively investigating. We have determined that WDF specifically prevents that call (WdfDeviceAssignProperty()) against a Child WDFDEVICE after creation. But we are investigating further to try to understand why that is the case. I will update this thread when I have more information.

Very interesting, good luck!

FYI: in the project I'm trying to replace my own verbose PDO code with Dmf_PDO I'm calling WdfDeviceAssignProperty in EvtWdfDeviceSelfManagedIoInit where it succeeds.

[samtertzakian]

Ok.... The issue you found is "by design" due to the fact that the underlying WDM layer has a rule about allowing properties to be set only after enumeration. The good news is that we have tested an update to the Module that does exactly that. So, this issue will be resolved in next update to DMF pending more validation on our end here.

As for your idea of making the change in EVT_DEVICE_SELF_MANAGED_IO_INIT, we can do that but then it prevents the Module from being loaded dynamically. But we have another solution as outlined above which allows the Module to be loaded dynamically.

Thank you for your feedback and idea above.

[nefarius]

Very interesting, awaiting the update! 👍

[samtertzakian]

Fix for this is under review. Thank you for your patience.

[samtertzakian]

Hi

This is the branch that we are testing with the fix for DMF_Pdo:

#219

We have verified that properties to child device are properly written.
We are just going through review and testing.
Can you kindly try this branch and verify it corrects your issue?

[nefarius]

Can you kindly try this branch and verify it corrects your issue?

Nice! Starting to test today.

[nefarius]

Alright, so it works partially, meaning that not all my properties get set, which could totally be an issue on my side, does this look like a valid way to build and pass the table?

Thanks!

"Old" code in my project

Using fixed branch version

[samtertzakian]

Ok, let me look at the above and get back to you.

[nefarius]

Looks like there's an issue in the data I provided:

DmfKModules.Library	Pdo_DevicePropertyTableWrite	2022/07/14-18:59:17.175	TRACE_LEVEL_WARNING	 [Pdo_DevicePropertyTableWrite]WdfDeviceAssignProperty fails: ntStatus=STATUS_INFO_LENGTH_MISMATCH (0xC0000004)

Looking at my code in parallel.

[samtertzakian]

We have to create a copy of the table since it is not used in caller's thread. We should verify that the table in the handler matches the table you passed in. There might be an error in the way we make a copy. We will hold off merging until your code works for sure.

[nefarius]

Am investigating, will update here ASAP, thanks for the quick fix!

[samtertzakian]

One thing...Generally speaking you don't want to create large data structures on the stack because the stack is limited for drivers. Stacks is shared among all drivers in the stack and the last time i checked, it is only 12k bytes. You should allocate the table dynamically, send it to PlugEx() and then free it so you don't create it on the stack. That is probably not the cause of the issue you have since your table is not massive, but still even for a table this size I would not create it on the stack.

[nefarius]

Nice reminder, will change my design!

[nefarius]

I think I found the issue; it looks like you're never advancing on the target table entries pointer so the loop is effectively always writing the last source property in the provided list in the first position of the target copy, then upon enumeration the 2nd element is just a zero block of memory.

[samtertzakian]

I am looking at that now....I am actually trying your table.

[samtertzakian]

Yes, you are correct. I see it now. Let me send an update.

[samtertzakian]

I have just updated the branch with the fix as well as updated test. We have reviewed the fix and we are rerunning regression tests. And I would like to wait until you verify the fix on your end. After that we need one more review of this exact merge here and then it will go to master.

Thank you again for your help and patience. Four of us reviewed the PR but we did not see that bug. We will try to be more careful.

[nefarius]

Aha! Brilliant, that's more like it:

All properties here!

Thank you again for your help and patience. Four of us reviewed the PR but we did not see that bug. We will try to be more careful.

Anytime, I've got so many driver projects I'd like to migrate to DMF I'll sure chime in again when I find things like this 😁 I'm happy to test and verify. Thanks for the quick responses and implementation, now I can throw away a ton of legacy code in my project and move on!

Cheers

[nefarius]

Btw. maybe this is related; I am using the IOCTL Handler Module in my PDO (which should be supported if I read the docs right) but it seems it suffers from the same design limitation (STATUS_INVALID_DEVICE_STATE on interface registration) like the property assignment?

DmfKModules.Library	DMF_IoctlHandler_IoctlStateSet	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [DMF_IoctlHandler_IoctlStateSet] --> Entry
DmfKModules.Library	IoctlHandler_DeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [IoctlHandler_DeviceInterfaceCreate] --> Entry
DmfKModules.Library	IoctlHandler_PostDeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [IoctlHandler_PostDeviceInterfaceCreate] --> Entry
DmfKModules.Library	IoctlHandler_PostDeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_ERROR	 [IoctlHandler_PostDeviceInterfaceCreate]WdfDeviceRetrieveDeviceInterfaceString fails: ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)
DmfKModules.Library	IoctlHandler_PostDeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [IoctlHandler_PostDeviceInterfaceCreate] <-- Exit <ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)>
DmfKModules.Library	IoctlHandler_DeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_ERROR	 [IoctlHandler_DeviceInterfaceCreate]IoctlHandler_PostDeviceInterfaceCreate fails: ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)
DmfKModules.Library	IoctlHandler_DeviceInterfaceCreate	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [IoctlHandler_DeviceInterfaceCreate] <-- Exit <ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)>
DmfKModules.Library	DMF_IoctlHandler_IoctlStateSet	2022/07/14-22:10:37.381	TRACE_LEVEL_ERROR	 [DMF_IoctlHandler_IoctlStateSet]IoctlHandler_DeviceInterfaceCreate fails: ntStatus=STATUS_INVALID_DEVICE_STATE (0xC0000184)
DmfKModules.Library	DMF_IoctlHandler_IoctlStateSet	2022/07/14-22:10:37.381	TRACE_LEVEL_VERBOSE	 [DMF_IoctlHandler_IoctlStateSet] <-- Exit

If so I'd open a separate issue (or maybe I used it wrong, this project's code base is quite big and I may have overlooked something).

[samtertzakian]

I verified a couple of days ago that setting creating the device interface for the property works (it worked before and it also works now after enumeration). But you are trying to set the state...and that is before enumeration...probably before it has been created. It looks like perhaps we may have to add a callback from within the WDM preprocessing callback so that you could set the device interface state. Do you want to try to create a callback in the PDO config that is called from the preprocessing callback?

By the way, the reason creating the device interface worked before is because WDF has code that specifically delays creating the interface until after enumeration.

[samtertzakian]

Let me try to reproduce your issue here and see what can be done.

[samtertzakian]

Hi Which properties work and which fail? On first glance your table looks correct to me.

…

On Thu, Jul 14, 2022, 2:34 AM Benjamin Höglinger-Stelzer < ***@***.***> wrote: Alright, so it works *partially*, meaning that not all my properties get set, which could totally be an issue on my side, does this look like a valid way to build and pass the table <https://github.com/ViGEm/BthPS3/blob/7e3aea27464579dc981d1ae8ac4dba307e6cbd3a/BthPS3/BusLogic.c#L1220-L1314> ? Thanks! — Reply to this email directly, view it on GitHub <#218 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHY5IWXPYCZDLUF4IB6RVRTVT7NIXANCNFSM52T57CJQ> . You are receiving this because you commented.Message ID: ***@***.***>