Skip to content

Commit

Permalink
Driver: fix IRP leak and client process hang
Browse files Browse the repository at this point in the history
When using SAR and with something using at least one of the endpoints,
killing the process that use SAR could cause the process to hang in a
unkillable state.
In that case, the process is hung waiting pending IRP against the SAR
driver to complete.

The issue appear when at least one of the endpoint is used, which cause
SarASIO to issue SAR_WAIT_HANDLE_QUEUE IOCTLs against the SAR driver.
SarASIO ensure that one IOCTL is always pending so if there is a handle
event on the driver side, SarASIO is triggered via the completion of the
pending SAR_WAIT_HANDLE_QUEUE IOCTL.
But in the event of a process kill, the process doesn't close its file
handle to the SAR driver cleanly, and instead file handles are closed by
windows itself.
When killing a process, open handles seems to be closed like this:
 - Issue a IRP_MJ_CLEANUP, which cause the orphaning of the SAR context
   context in the driver (SarOrphanControlContext)
 - Loop all pending IRPs and cancel them. At this point, the cancel
   function of the SAR driver does nothing as the control context was
   released already.
   So pending IRPs are leaked and never completed.
 - Windows wait the canceled IRPs to complete (with a status
   STATUS_PENDING)

The root cause of all of this, is that when releasing the SAR
driver control context on IRP_MJ_CLEANUP or CLOSE, pending IRPs are leaked
and never completed.

This commit fix that and cancel all pending IRPs when orphaning the
control context.

As a side note, the modification of SarDeleteEndpoint fix a crash when
endpoint->filterFactory is NULL (which can happen in case of a failure in
SarCreateEndpoint before the filterFactory is initialized. Detected with
the driver verifier tool.

Fix issue #59.
  • Loading branch information
amurzeau committed Dec 30, 2018
1 parent d4d5423 commit 15b8ea8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 1 deletion.
3 changes: 2 additions & 1 deletion SynchronousAudioRouter/control.cpp
Expand Up @@ -772,9 +772,10 @@ NTSTATUS SarCreateEndpoint(

VOID SarDeleteEndpoint(SarEndpoint *endpoint)
{
PKSDEVICE ksDevice = KsFilterFactoryGetDevice(endpoint->filterFactory);

if (endpoint->filterFactory) {
PKSDEVICE ksDevice = KsFilterFactoryGetDevice(endpoint->filterFactory);

KsAcquireDevice(ksDevice);
KsDeleteFilterFactory(endpoint->filterFactory);
KsReleaseDevice(ksDevice);
Expand Down
2 changes: 2 additions & 0 deletions SynchronousAudioRouter/entry.cpp
Expand Up @@ -152,6 +152,8 @@ BOOLEAN SarOrphanControlContext(SarDriverExtension *extension, PIRP irp)
SarOrphanEndpoint(endpoint);
}

SarCancelAllHandleQueueIrps(&controlContext->handleQueue);

SarReleaseControlContext(controlContext);
return TRUE;
}
Expand Down
1 change: 1 addition & 0 deletions SynchronousAudioRouter/sar.h
Expand Up @@ -470,6 +470,7 @@ void SarInitializeHandleQueue(SarHandleQueue *queue);
NTSTATUS SarTransferQueuedHandle(
PIRP irp, HANDLE kernelTargetProcessHandle, ULONG responseIndex,
HANDLE kernelProcessHandle, HANDLE userHandle, ULONG64 associatedData);
void SarCancelAllHandleQueueIrps(SarHandleQueue *handleQueue);
void SarCancelHandleQueueIrp(PDEVICE_OBJECT deviceObject, PIRP irp);
NTSTATUS SarPostHandleQueue(
SarHandleQueue *queue, HANDLE userHandle, ULONG64 associatedData);
Expand Down
36 changes: 36 additions & 0 deletions SynchronousAudioRouter/utility.cpp
Expand Up @@ -257,6 +257,42 @@ NTSTATUS SarTransferQueuedHandle(
return status;
}

void SarCancelAllHandleQueueIrps(SarHandleQueue *handleQueue)
{
KIRQL irql;
LIST_ENTRY pendingIrqsToCancel;

InitializeListHead(&pendingIrqsToCancel);

KeAcquireSpinLock(&handleQueue->lock, &irql);

if (!IsListEmpty(&handleQueue->pendingIrps)) {
PLIST_ENTRY entry = handleQueue->pendingIrps.Flink;

RemoveEntryList(&handleQueue->pendingIrps);
InitializeListHead(&handleQueue->pendingIrps);
AppendTailList(&pendingIrqsToCancel, entry);
}

KeReleaseSpinLock(&handleQueue->lock, irql);

while (!IsListEmpty(&pendingIrqsToCancel)) {
SarHandleQueueIrp *pendingIrp =
CONTAINING_RECORD(pendingIrqsToCancel.Flink, SarHandleQueueIrp, listEntry);
PIRP irp = pendingIrp->irp;

RemoveEntryList(&pendingIrp->listEntry);

ZwClose(pendingIrp->kernelProcessHandle);
ExFreePoolWithTag(pendingIrp, SAR_TAG);

irp->IoStatus.Information = 0;
irp->IoStatus.Status = STATUS_CANCELLED;
IoSetCancelRoutine(irp, nullptr);
IoCompleteRequest(irp, IO_NO_INCREMENT);
}
}

void SarCancelHandleQueueIrp(PDEVICE_OBJECT deviceObject, PIRP irp)
{
UNREFERENCED_PARAMETER(deviceObject);
Expand Down

0 comments on commit 15b8ea8

Please sign in to comment.