[PATCH 1/5] ntoskrnl: Implement IoSetDevicePropertyData().

[Rémi Bernon]

On 5/4/21 9:11 PM, Zebediah Figura (she/her) wrote:
> On 5/4/21 12:37 PM, Rémi Bernon wrote:
>> On 4/26/21 6:28 PM, Zebediah Figura (she/her) wrote:
>>> On 4/26/21 5:37 AM, Rémi Bernon wrote:
>>>> From: Arkadiusz Hiler <ahiler at codeweavers.com>
>>>>
>>>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>>>> ---
>>>>
>>>> In PATCH 1, the return GetLastError() look wrong to me but the same is
>>>> done in IoGetDeviceProperty so I kept them. I guess we would need to
>>>> convert the setupapi error back to NTSTATUS, or implement the higher
>>>> level setupapi on top of these lower level primitives intead.
>>>
>>> It's maybe not exactly ideal, but it's also not clear how to map
>>> setupapi errors to NTSTATUS, and moreover it probably doesn't matter
>>> anyway (we don't expect those functions to fail).
>>>
>>> I don't think we can implement setupapi functions on top of ntoskrnl
>>> ones; that crosses the kernel boundary (and there's no clear syscall
>>> interface for it). Probably on Windows the code is just duplicated.
>>>
>>> FWIW, we have a PnP test driver now, so it's actually possible to write
>>> tests for this function.
>>>
>>>>
>>>> In PATCH 2, I moved the error case before IoInvalidateDeviceRelations,
>>>> because I don't know what the call is actually expecting. Maybe it was
>>>> unnecessary.
>>>
>>> I think it's correct as-is to move it—as soon as we call
>>> IoInvalidateDeviceRelations() the device can be exposed.
>>>
>>> Actually, to be completely correct (i.e. if we were running this driver
>>> in Windows), I think we should treat it as possible to receive
>>> IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be
>>> doing is to initialize everything necessary *before* the statement
>>> "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at
>>> IRP_MN_START_DEVICE (on the child PDO).
>>>
>>
>> It's actually causing IoSetDevicePropertyData to fail the first time a
>> device is detected (so on prefix creation for both mouse and keyboard
>> devices), as setupapi doesn't know about it until we call
>> IoInvalidateDeviceRelations.
>>
>> Note however, although it fails to set the property, as
>> SetupDiDestroyDeviceInfoList clears the last error, it still returns
>> success, and the device initialization continues.
>>
>> I think it may be better to move it after, as it's less likely that
>> anything happens in between, while missing the property is going to make
>> rawinput device enumeration skip the device. But it's just a workaround.
>>
> 
> Sorry, can you please clarify—what's causing the initial call to fail?
> 

IoSetDevicePropertyData fails, because SetupDiOpenDeviceInfoW fails, 
because, I think, the registry keys haven't been created yet (it returns 
ERROR_NO_SUCH_DEVINST).

My understanding (although I didn't investigate much) is that 
IoInvalidateDeviceRelations creates the registry keys if the device has 
not been seen before.

The symptoms are these messages being printed when a device is first 
seen (so on initial prefix creation for mouse and keyboard devices):

err:plugplay:IoSetDevicePropertyData Failed to open device, error 
0xe000020b.
-- 
Rémi Bernon <rbernon at codeweavers.com>