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.