[PATCH v2 1/2] ntoskrnl.exe: Implement IoRegisterDeviceInterface

[Zebediah Figura]

On 03/10/18 10:02, Aric Stewart wrote:
> +static NTSTATUS get_instance_id(DEVICE_OBJECT *device, WCHAR
**instance_id)
> +{
> +    WCHAR *id, *ptr;
> +    NTSTATUS status;
> +
> +    status = get_device_id( device, BusQueryInstanceID, &id );
> +    if (status != STATUS_SUCCESS)
> +    {
> +        FIXME( "failed to get device ID\n" );
> +        return status;
> +    }

Would ERR be more appropriate?

> +
> +    struprW( id );
> +    ptr = strchrW( id, '\\' );
> +
> +    while (ptr)
> +    {
> +        *ptr = '#';
> +        ptr = strchrW( id,'\\' );
> +    }

Stylistic choice, perhaps, but maybe this would be simpler?

while ((ptr = strchrW( id, '\\' )))
    *ptr = '#';

> +
> +    *instance_id = id;
> +    return STATUS_SUCCESS;
> +}
> +
> +
> +/*****************************************************
> + *           IoRegisterDeviceInterface(NTOSKRNL.EXE.@)
> + */
> +NTSTATUS WINAPI IoRegisterDeviceInterface(DEVICE_OBJECT *device,
const GUID *class_guid, UNICODE_STRING *reference_string, UNICODE_STRING
*symbolic_link)
> +{
> +    WCHAR *instance_id;
> +    NTSTATUS status = STATUS_SUCCESS;
> +    HDEVINFO infoset;
> +    WCHAR *referenceW = NULL;
> +    SP_DEVINFO_DATA devInfo;
> +    SP_DEVICE_INTERFACE_DATA infoData;
> +    SP_DEVICE_INTERFACE_DETAIL_DATA_W *data;
> +    BOOL rc;
> +
> +    TRACE( "(%p, %s, %s, %p)\n", device, debugstr_guid(class_guid),
debugstr_us(reference_string), symbolic_link );
> +
> +    if (reference_string != NULL)
> +        referenceW = reference_string->Buffer;
> +
> +    infoset = SetupDiGetClassDevsW(class_guid, referenceW, NULL,
DIGCF_DEVICEINTERFACE);
> +
> +    status = get_instance_id( device, &instance_id );
> +    if (status != STATUS_SUCCESS)
> +    {
> +        ERR( "Failed to generate Instance ID\n" );
> +        return status;
> +    }
> +
> +    devInfo.cbSize = sizeof(devInfo);
> +    rc = SetupDiCreateDeviceInfoW( infoset, instance_id, class_guid,
NULL, NULL, 0, &devInfo );
> +    if (rc == 0)
> +    {
> +        if (GetLastError() == ERROR_DEVINST_ALREADY_EXISTS)
> +        {
> +            DWORD required;
> +            DWORD index = 0;
> +            DWORD size = strlenW(instance_id) + 2;
> +            WCHAR *id = HeapAlloc(GetProcessHeap(), 0, size *
sizeof(WCHAR));
> +            do
> +            {
> +                rc = SetupDiEnumDeviceInfo( infoset, index, &devInfo );
> +                if (IsEqualGUID(&devInfo.ClassGuid, class_guid))
> +                {
> +                    BOOL check;
> +                    check = SetupDiGetDeviceInstanceIdW(infoset,
&devInfo, id, size, &required);
> +                    if (check && strcmpW(id, instance_id) == 0)
> +                        break;
> +                }
> +                index++;
> +            } while (rc);

I guess it maybe shouldn't block this patch, but it would be good to
implement SetupDiOpenDeviceInfo() at some point; it would result in much
cleaner code on both sides.

> +
> +            HeapFree(GetProcessHeap(), 0, id);
> +            if (!rc)
> +            {
> +                ERR( "Failed to find Device\n" );
> +                HeapFree( GetProcessHeap(), 0, instance_id );
> +                return STATUS_UNSUCCESSFUL;
> +            }
> +        }
> +        else
> +        {
> +            ERR( "Failed to Create Device\n" );
> +            HeapFree( GetProcessHeap(), 0, instance_id );
> +            return STATUS_UNSUCCESSFUL;
> +        }
> +    }
> +    HeapFree( GetProcessHeap(), 0, instance_id );
> +
> +    infoData.cbSize = sizeof(infoData);
> +    rc = SetupDiCreateDeviceInterfaceW(infoset, &devInfo, class_guid,
NULL, 0, &infoData);
> +    if (!rc)
> +    {
> +        ERR("Failed to create device interface\n");
> +        return STATUS_UNSUCCESSFUL;
> +    }
> +
> +    data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY ,
sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) + 1000);

This seems kind of ugly; can't we just call it twice?

> +    data->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W);
> +
> +    rc = SetupDiGetDeviceInterfaceDetailW(infoset, &infoData, data,
500, NULL, NULL);
> +    if (!rc)
> +    {
> +        ERR("Failed Get device details\n");
> +        return STATUS_UNSUCCESSFUL;
> +    }
> +
> +    data->DevicePath[1] = '?';
> +    ERR("Device path %s\n",debugstr_w(data->DevicePath));

Leftover debug string?

> +
> +    if (symbolic_link)
> +        RtlCreateUnicodeString( symbolic_link, data->DevicePath);
> +
> +    HeapFree( GetProcessHeap(), 0, data );
> +
> +    return status;
> +}
> +
> +
>  /***********************************************************************
>   *           IoRegisterDriverReinitialization    (NTOSKRNL.EXE.@)
>   */
> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> index 422d575926..1a6139de61 100644
> --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> @@ -428,7 +428,7 @@
>  @ stub IoReadPartitionTableEx
>  @ stub IoReadTransferCount
>  @ stub IoRegisterBootDriverReinitialization
> -@ stub IoRegisterDeviceInterface
> +@ stdcall IoRegisterDeviceInterface(ptr ptr ptr ptr)
>  @ stdcall IoRegisterDriverReinitialization(ptr ptr ptr)
>  @ stdcall IoRegisterFileSystem(ptr)
>  @ stub IoRegisterFsRegistrationChange
> diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h
> index 0cd1673dbe..cd057d5a9f 100644
> --- a/include/ddk/wdm.h
> +++ b/include/ddk/wdm.h
> @@ -1405,6 +1405,7 @@ PDEVICE_OBJECT WINAPI
IoGetRelatedDeviceObject(PFILE_OBJECT);
>  void      WINAPI IoInitializeIrp(IRP*,USHORT,CCHAR);
>  VOID      WINAPI
IoInitializeRemoveLockEx(PIO_REMOVE_LOCK,ULONG,ULONG,ULONG,ULONG);
>  void      WINAPI
IoInvalidateDeviceRelations(PDEVICE_OBJECT,DEVICE_RELATION_TYPE);
> +NTSTATUS  WINAPI IoRegisterDeviceInterface(PDEVICE_OBJECT,const
GUID*,PUNICODE_STRING,PUNICODE_STRING);
>  void      WINAPI IoReleaseCancelSpinLock(KIRQL);
>  NTSTATUS  WINAPI IoSetDeviceInterfaceState(UNICODE_STRING*,BOOLEAN);
>  NTSTATUS  WINAPI IoWMIRegistrationControl(PDEVICE_OBJECT,ULONG);