MULTIPLE_IRP_COMPLETE_REQUESTS

NTDEV
1

Hello,

I have an existing driver wrote some years ago where I was getting a B8
BSOD because the driver tried to synchronize access to some internal
structures in a DPC thread during a completion routine.

I changed it so the completion routine now queues an work item using
IoQueueWorkItem
and I do my stuff in a lower IRQL where I am able to synchronize.
Now the B8 BSOD is gone, but I am getting eventually a
MULTIPLE_IRP_COMPLETE_REQUESTS BSOD and I don’t know why.

I am sending below part of the code I changed.
Does anynone can help me?

Thanks,

typedef struct {UCHAR Operation;PIRP Irp;PDEVICE_OBJECT Device;CAttachedDevice *AttachedDevice;} *PNGWORKITEM, NGWORKITEM;VOID ProcessWorkItem(PDEVICE_OBJECT DeviceObject, PVOID Context){LOG_TRACE_ENTERING;PNGWORKITEM ngWorkItem = (PNGWORKITEM)Context;if (ngWorkItem->Irp->PendingReturned) { IoMarkIrpPending(ngWorkItem->Irp); }PIO_STACK_LOCATION stackLocation =IoGetCurrentIrpStackLocation(ngWorkItem->Irp);// Create our IO request structure.IOReq* req = new (NonPagedPool) IOReq(ngWorkItem->Operation == 'R' ? REQ_READ : REQ_WRITE,ngWorkItem->Operation == 'R' ? stackLocation->Parameters.Read.Length :stackLocation->Parameters.Write.Length,(ULONG)ngWorkItem->Irp->IoStatus.Information,ngWorkItem->Irp->AssociatedIrp.SystemBuffer);// If it was possible to allocate an IO request, add it to the list.if (NULL != req) {ngWorkItem->AttachedDevice->New(req);}// Complete the request.IoCompleteRequest(ngWorkItem->Irp,IO_NO_INCREMENT);// Release the memory we allocated.ExFreePool(Context);LOG_TRACE_LEAVING_VOID;}NTSTATUS QueueWorkItem(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp, IN UCHAR Operation,IN CAttachedDevice *CADevice){PIO_WORKITEM workItem = IoAllocateWorkItem(DeviceObject);if (NULL == workItem) {return STATUS_SUCCESS;}PNGWORKITEM ngWorkItem = (PNGWORKITEM)ExAllocatePool(NonPagedPool,sizeof(NGWORKITEM));if (NULL == ngWorkItem) {return STATUS_SUCCESS;}ngWorkItem->Device = DeviceObject;ngWorkItem->AttachedDevice = CADevice;ngWorkItem->Irp = Irp;ngWorkItem->Operation = Operation;IoQueueWorkItem(workItem,ProcessWorkItem,DelayedWorkQueue,ngWorkItem);return STATUS_MORE_PROCESSING_REQUIRED;}NTSTATUS ReadCompletion( IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp, IN PVOID Context){return QueueWorkItem(DeviceObject, Irp, 'R', (CAttachedDevice *)Context);}NTSTATUS WriteCompletion(IN PDEVICE_OBJECT DeviceObject,IN PIRP Irp,IN PVOID Context){return QueueWorkItem(DeviceObject, Irp, 'W', (CAttachedDevice *)Context);}NTSTATUS CAttachedDevice::Standard( /// I/O request packet identifier. PIRP Irp, /// Completion PIO_COMPLETION_ROUTINE Routine){ LOG_TRACE_ENTERING;// This function forwards the request. PIO_STACK_LOCATION curIRPStack, nextIRPStack;curIRPStack = IoGetCurrentIrpStackLocation(Irp); nextIRPStack = IoGetNextIrpStackLocation(Irp); *nextIRPStack = *curIRPStack;IoSetCompletionRoutine( Irp, (Routine) ? Routine : DefaultCompletion, this, TRUE, TRUE, TRUE);NTSTATUS status = IoCallDriver(OriginalDevice, Irp);LOG_TRACE_LEAVING_STATUS;return status;}

–
-George

2

The fact that this is in WDM is unfortunate. You really should consider
re-writing this as a WDF driver, it will save you from knowing a bunch of
arcane IRP handling details.

There are at least a couple of incorrect details here. This code:

if (ngWorkItem->Irp->PendingReturned) {
IoMarkIrpPending(ngWorkItem->Irp);
}

Needs to be inline in your completion routine, not in a work item queued by
the completion routine. If you do it in a work item it’s too late for the
code to actually do anything useful.

But, you shouldn’t have that code anyway because you’re returning
STATUS_MORE_PROCESSING_REQUIRED from your completion routine and therefore
you should have already marked the IRP as pending in your dispatch entry
point. The way the code is currently structured will definitely lead to this
exact BSOD. Do this instead:

IoMarkIrpPending(Irp);

(VOID)IoCallDriver(OriginalDevice, Irp);

return STATUS_PENDING;

Also make sure you run your driver under Driver Verifier as it hopefully
would have caught this earlier.

And it’s worth mentioning again: seriously consider rewriting this in WDF.
Whatever pain you go through during the rewrite will pay off in the long
run.

-scott
OSR
@OSRDrivers

“George Luiz Bittencourt” wrote in message
news:xxxxx@ntdev…
Hello,

I have an existing driver wrote some years ago where I was getting a B8 BSOD
because the driver tried to synchronize access to some internal structures
in a DPC thread during a completion routine.

I changed it so the completion routine now queues an work item using
IoQueueWorkItem and I do my stuff in a lower IRQL where I am able to
synchronize.
Now the B8 BSOD is gone, but I am getting eventually a
MULTIPLE_IRP_COMPLETE_REQUESTS BSOD and I don’t know why.

I am sending below part of the code I changed.
Does anynone can help me?

Thanks,



typedef struct {
UCHAR Operation;
PIRP Irp;
PDEVICE_OBJECT Device;
CAttachedDevice *AttachedDevice;
} PNGWORKITEM, NGWORKITEM;

VOID ProcessWorkItem(PDEVICE_OBJECT DeviceObject, PVOID Context)
{
LOG_TRACE_ENTERING;

PNGWORKITEM ngWorkItem = (PNGWORKITEM)Context;

if (ngWorkItem->Irp->PendingReturned) {
IoMarkIrpPending(ngWorkItem->Irp);
}

PIO_STACK_LOCATION stackLocation =
IoGetCurrentIrpStackLocation(ngWorkItem->Irp);

// Create our IO request structure.
IOReq req = new (NonPagedPool) IOReq(
ngWorkItem->Operation == ‘R’ ? REQ_READ : REQ_WRITE,
ngWorkItem->Operation == ‘R’ ? stackLocation->Parameters.Read.Length :
stackLocation->Parameters.Write.Length,
(ULONG)ngWorkItem->Irp->IoStatus.Information,
ngWorkItem->Irp->AssociatedIrp.SystemBuffer
);

// If it was possible to allocate an IO request, add it to the list.
if (NULL != req) {
ngWorkItem->AttachedDevice->New(req);
}

// Complete the request.
IoCompleteRequest(
ngWorkItem->Irp,
IO_NO_INCREMENT
);

// Release the memory we allocated.
ExFreePool(
Context
);

LOG_TRACE_LEAVING_VOID;
}

NTSTATUS QueueWorkItem
(
IN PDEVICE_OBJECT DeviceObject,
IN PIRP Irp,
IN UCHAR Operation,
IN CAttachedDevice *CADevice
)
{
PIO_WORKITEM workItem = IoAllocateWorkItem(
DeviceObject
);

if (NULL == workItem) {
return STATUS_SUCCESS;
}

PNGWORKITEM ngWorkItem = (PNGWORKITEM)ExAllocatePool(
NonPagedPool,
sizeof(NGWORKITEM)
);

if (NULL == ngWorkItem) {
return STATUS_SUCCESS;
}

ngWorkItem->Device = DeviceObject;
ngWorkItem->AttachedDevice = CADevice;
ngWorkItem->Irp = Irp;
ngWorkItem->Operation = Operation;

IoQueueWorkItem(
workItem,
ProcessWorkItem,
DelayedWorkQueue,
ngWorkItem
);

return STATUS_MORE_PROCESSING_REQUIRED;
}

NTSTATUS ReadCompletion
(
IN PDEVICE_OBJECT DeviceObject,
IN PIRP Irp,
IN PVOID Context
)
{
return QueueWorkItem(DeviceObject, Irp, ‘R’, (CAttachedDevice *)Context);
}

NTSTATUS WriteCompletion
(
IN PDEVICE_OBJECT DeviceObject,
IN PIRP Irp,
IN PVOID Context
)
{
return QueueWorkItem(DeviceObject, Irp, ‘W’, (CAttachedDevice *)Context);
}

NTSTATUS CAttachedDevice::Standard
(
/// I/O request packet identifier.
PIRP Irp,
/// Completion
PIO_COMPLETION_ROUTINE Routine
)
{
LOG_TRACE_ENTERING;

// This function forwards the request.
PIO_STACK_LOCATION curIRPStack, nextIRPStack;

curIRPStack = IoGetCurrentIrpStackLocation(Irp);
nextIRPStack = IoGetNextIrpStackLocation(Irp);
*nextIRPStack = *curIRPStack;

IoSetCompletionRoutine(
Irp,
(Routine) ? Routine : DefaultCompletion,
this,
TRUE,
TRUE,
TRUE);

NTSTATUS status = IoCallDriver(OriginalDevice, Irp);

LOG_TRACE_LEAVING_STATUS;

return status;
}



–

-George

3

George Luiz Bittencourt wrote:

…
VOID ProcessWorkItem(PDEVICE_OBJECT DeviceObject, PVOID Context)
{
…
// Create our IO request structure.
IOReq* req = new (NonPagedPool) IOReq(
ngWorkItem->Operation == ‘R’ ? REQ_READ : REQ_WRITE,
ngWorkItem->Operation == ‘R’ ? stackLocation->Parameters.Read.Length :
stackLocation->Parameters.Write.Length,
(ULONG)ngWorkItem->Irp->IoStatus.Information,
ngWorkItem->Irp->AssociatedIrp.SystemBuffer
);

// If it was possible to allocate an IO request, add it to the list.
if (NULL != req) {
ngWorkItem->AttachedDevice->New(req);
}

// Complete the request.
IoCompleteRequest(
ngWorkItem->Irp,
IO_NO_INCREMENT
);

This is dangerous. You are saving the SystemBuffer address from the IRP
in your IOReq and then immediately completing the IRP. But once you
complete the IRP, that SystemBuffer address is no longer valid. The I/O
manager will free the memory.

Also, I suspect you want to do the IoMarkIrpPending before your
completion routine returns, although it may be the operating system
doesn’t actually care about that.

–
Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

4

Tim and Scott,

Thanks for your help!

-George

On Thu, Apr 20, 2017 at 12:46 PM, Tim Roberts wrote:

> George Luiz Bittencourt wrote:
> > …
> > VOID ProcessWorkItem(PDEVICE_OBJECT DeviceObject, PVOID Context)
> > {
> > …
> > // Create our IO request structure.
> > IOReq* req = new (NonPagedPool) IOReq(
> > ngWorkItem->Operation == ‘R’ ? REQ_READ : REQ_WRITE,
> > ngWorkItem->Operation == ‘R’ ? stackLocation->Parameters.Read.Length :
> > stackLocation->Parameters.Write.Length,
> > (ULONG)ngWorkItem->Irp->IoStatus.Information,
> > ngWorkItem->Irp->AssociatedIrp.SystemBuffer
> > );
> >
> > // If it was possible to allocate an IO request, add it to the list.
> > if (NULL != req) {
> > ngWorkItem->AttachedDevice->New(req);
> > }
> >
> > // Complete the request.
> > IoCompleteRequest(
> > ngWorkItem->Irp,
> > IO_NO_INCREMENT
> > );
>
> This is dangerous. You are saving the SystemBuffer address from the IRP
> in your IOReq and then immediately completing the IRP. But once you
> complete the IRP, that SystemBuffer address is no longer valid. The I/O
> manager will free the memory.
>
> Also, I suspect you want to do the IoMarkIrpPending before your
> completion routine returns, although it may be the operating system
> doesn’t actually care about that.
>
> –
> Tim Roberts, xxxxx@probo.com
> Providenza & Boekelheide, Inc.
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> 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;
>

–
-George</http:></http:>