Re: [PATCH v3 3/4] winexinput.sys: Introduce a new winexinput.sys driver.

[Zebediah Figura (she/her)]

Sorry I missed this before, but...

On 8/30/21 4:06 AM, Rémi Bernon wrote:
> +static NTSTATUS WINAPI pdo_pnp(DEVICE_OBJECT *device, IRP *irp)
> +{
> - IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp);
> - struct func_device *fdo = fdo_from_DEVICE_OBJECT(device);
> - struct device *impl = impl_from_DEVICE_OBJECT(device);
> - ULONG code = stack->MinorFunction;
> - NTSTATUS status;
> -
> - TRACE("device %p, irp %p, code %#x, bus_device %p.\n", device, irp, code, fdo->bus_device);
> -
> - switch (code)
> - {
> - case IRP_MN_START_DEVICE:
> -
>    status = STATUS_SUCCESS;
> -
>    break;
> -
> - case IRP_MN_SURPRISE_REMOVAL:
> -
>    status = STATUS_SUCCESS;
> -
>    if (InterlockedExchange(&impl->removed, TRUE)) break;
> -
>    break;
> -
> - case IRP_MN_REMOVE_DEVICE:
> -
>    irp->IoStatus.Status = STATUS_SUCCESS;
> -
>    IoCompleteRequest(irp, IO_NO_INCREMENT);
> -
>    IoDeleteDevice(device);
> -
>    return STATUS_SUCCESS;
> -
> - case IRP_MN_QUERY_ID:
> -
>    switch (stack->Parameters.QueryId.IdType)
> -
>    {
> -
>    case BusQueryHardwareIDs:
> -
>        irp->IoStatus.Information = (ULONG_PTR)query_hardware_ids(device);
> -
>        break;
> -
>    case BusQueryCompatibleIDs:
> -
>        irp->IoStatus.Information = (ULONG_PTR)query_compatible_ids(device);
> -
>        break;
> -
>    case BusQueryDeviceID:
> -
>        irp->IoStatus.Information = (ULONG_PTR)query_device_id(device);
> -
>        break;
> -
>    case BusQueryInstanceID:
> -
>        irp->IoStatus.Information = (ULONG_PTR)query_instance_id(device);
> -
>        break;
> -
>    default:
> -
>        IoSkipCurrentIrpStackLocation(irp);
> -
>        return IoCallDriver(fdo->bus_device, irp);
> -
>    }
> -
> -
>    if (!irp->IoStatus.Information) status = STATUS_NO_MEMORY;
> -
>    else status = STATUS_SUCCESS;
> -
>    break;
> -
> - default:
> -
>    IoSkipCurrentIrpStackLocation(irp);
> -
>    return IoCallDriver(fdo->bus_device, irp);

...this is wrong; you can't pass PnP IRPs down to the parent device 
stack. You should be completing them with the status already in 
irp->IoStatus.Status instead.

For the record, I think it's a good idea to also print a FIXME in this 
case—just in case we start sending out a new IRP from ntoskrnl that our 
drivers need to handle.
> - }
> -
> - irp->IoStatus.Status = status;
> - IoCompleteRequest(irp, IO_NO_INCREMENT);
> - return status;
>
> +}
> -

Otherwise I think this patch looks good.