The 'right' thing to do

I pulled out an age old filter for a project on win10IoT Enterprise and had an interesting Verifier error, setting the status of an IoStatus Block the driver didn’t ‘understand’. It was actually an IRP_MN_QUERY_DEVICE_RELATIONS Irp, which is handled synchronously, with the completion routine returning ‘more processing’ and the Irp completed later. The status returned from IoCallDriver() is put into the IoStatus Block and that status returned from the Irp handler.

(This filter driver has been used from the days of XP, been signed many times in various guises, and run through Verifier many many times. )

Yet now its wrong. Apparently the status from IoCallDriver() should be ignored and instead the status read from the IoStatus block and THIS status returned from the Irp handler.

As I said to a fellow engineer some years back, the ‘right’ thing to do is defined by what Verifier accepts…

The rules are simple. If you complete an IRP in your dispatch routine, the return MUST match IoStatus.Status, UNLESS you return STATUS_PENDING, in which case you MUST call IoMarkIrpPending. If you DON’T complete the IRP in your dispatch routine, calling IoCallDriver for the lower driver, you MUST return whatever IoCallDriver returns, UNLESS you call IoMarkIrpPending before IoCallDriver, in which case you MUST return STATUS_PENDING.

Quite. Now we have a new verifier clarifying the situation we can clearly see that this:


Irp->IoStatus.Status = status;
IoCompleteRequest (Irp, IO_NO_INCREMENT);
return status;

from the filter sample code in the 7600 WDK is in fact wrong and should be:


status = Irp->IoStatus.Status;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
return status;

Hold on - why do you mention IoCompleteRequest() if you are passing an IRP down the stack???

IIRC, there are just 2 cases when you may call this function in a filter driver:

  1. You complete an IRP without passing it down the stack

  2. Your IO completion routine returns STATUS_MORE_PROCESSING_REQUIRED, which immediately terminates the callback chain of IO completion routines for a given IRP. In this case you MUST call IoCompleteRequest() at some later point so that IO completion callback chain gets resumed

Anton Bassov

Mr. Grigg’s stated the rule exactly correctly. That’s how we teach it in class.

In the “block in pre-processing, waiting for an event set in the completion routine” case… the code you show is right.

It’s also important to note that, by implementation if not by architecture, it doesn’t matter. The I/O Manager just cares if the status returned is STATUS_PENDING or not.

That why the code has worked for these past hundred or so years.

Verifier is being overly fussy here… thanks for bringing this issue to the community’s attention Mr, Sykes. Very useful.

Peter
OSR
@OSRDrivers

@Anton #2.

@Peter, yes, I guess the IO manager is pretty ambivalent about this since its always worked OK.

Hopefully there aren’t any other Win10 verifier foibles…

I decided to review some boilerplate code that I have been using for a
while. It looks like I have done it correctly.

// Generic synchronous completion routine.
NTSTATUS _IrpSyncCompletion(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp,
PKEVENT Event) {
if (Irp->PendingReturned) {
if (ARGUMENT_PRESENT(Event)) {
KeSetEvent(Event, IO_NO_INCREMENT, FALSE);
}
}
return STATUS_MORE_PROCESSING_REQUIRED;
UNREFERENCED_PARAMETER(DeviceObject);
}

NTSTATUS _CallDriverSynchronous(IN PDEVICE_OBJECT DeviceObject, IN PIRP
Irp) {
KEVENT event;
KeInitializeEvent(&event, NotificationEvent, FALSE);
IoCopyCurrentIrpStackLocationToNext(Irp);
IoSetCompletionRoutine(Irp, _IrpSyncCompletion, &event, TRUE, TRUE, TRUE);
NTSTATUS status = IoCallDriver(DeviceObject, Irp);
if (STATUS_PENDING == status) {
KeWaitForSingleObject(&event, Executive, KernelMode, FALSE, NULL);
status = Irp->IoStatus.Status;
}
return status;
}

On Thu, Nov 24, 2016 at 9:31 AM wrote:

> @Peter, yes, I guess the IO manager is pretty ambivalent about this since
> its always worked OK.
>
> Hopefully there aren’t any other Win10 verifier foibles…
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: <
> http://www.osronline.com/showlists.cfm?list=ntdev&gt;
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:>

Why do you even need to call the lower driver synchronously, if you just complete the IRP immediately?

I don’t have 7600 WDK handy. What was the actual code there?

This was a legit bug and the verifier caught it.

@Alex Its the handling of query relations. The filter hangs on to the Irp to get the PDO list then completes it.

That code snippet comes out of the toaster filter sample, and like many filters, has been copied, including the one I have, hence verifier barfing.

It is an excessively pedantic verifier catch though, the lower driver that does the work sets the status in the Io block and returns the same stats, which is passed up the stack so when the filter (now mistakenly) sets the Io block status to the same as status, its actually the same value. (The only exception is pending from the lower driver, the filter then waits and copies the status out of the Io block. ).

So in fact its all the same anyway.

Oh well. Like I said, the right thing to do is what verifier tells you to do, the documentation isn’t detailed enough, because here we are, many years later, still going over the details of Irp handling…