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

[Zebediah Figura (she/her)]

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?