Hesham/wintun
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't cancel pending IRPs on adapter pause

Browse Source 
Pausing an adapter requires it to stop processing network traffic. We
may still process I/O of the adapter device pipe.

By canceling all pending IRPs and refusing to serve any new IRPs on
adapter pause, we forcibly cut the client off to release resources and
allow graceful driver unload. This enabled a hot driver update without a
restart.

However, using a modified Wintun driver for NDISTEST utility the client
didn't behave the same way, keeping handles open when NDISTEST paused
Wintun adapter. The pause reset ctx->Device.RefCount to 0 which caused
negative reference count when client finally decided to close the Wintun
adapter device pipe handle.

Rather than introducing a complex fix to the adapter device pipe
reference counting, this commit simplifies it by allowing IRPs even when
adapter is paused. IRP_MJ_WRITE is an exception: client cannot write to
the adapter when it's paused.

This allows the client to stay connected, reduces ActiveTransactionCount
to NBL counting only, and cancels pending IRPs much later in
TunHaltEx().

Since there is still I/O going on after TunHaltEx() returns now, the
driver is not unloaded from memory. This prevents driver updates without
a reboot. How can one gracefully terminate I/O in TunHaltEx() remains a
subject of further study.

Signed-off-by: Simon Rozman <simon@rozman.si>
This commit is contained in:
Simon Rozman 2019-04-05 20:11:08 +02:00
parent 7176236b4e
commit 1369525e3a
1 changed files with 26 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
wintun.c
View File

@ -168,10 +168,10 @@ static void TunCompleteRequest(_Inout_ IRP *Irp, _In_ ULONG_PTR Information, _In
_IRQL_requires_same_ _IRQL_requires_same_
_Must_inspect_result_ _Must_inspect_result_
static NTSTATUS TunCheckForPause(_Inout_ TUN_CTX *ctx, _In_ LONG64 increment) static NTSTATUS TunCheckForPause(_Inout_ TUN_CTX *ctx)
{ {
ASSERT(InterlockedGet64(&ctx->ActiveTransactionCount) <= MAXLONG64 - increment); ASSERT(InterlockedGet64(&ctx->ActiveTransactionCount) < MAXLONG64);
InterlockedAdd64(&ctx->ActiveTransactionCount, increment); InterlockedIncrement64(&ctx->ActiveTransactionCount);
return return
InterlockedGet((LONG *)&ctx->State) != TUN_STATE_RUNNING ? STATUS_NDIS_PAUSED : InterlockedGet((LONG *)&ctx->State) != TUN_STATE_RUNNING ? STATUS_NDIS_PAUSED :
InterlockedGet((LONG *)&ctx->PowerState) >= NdisDeviceStateD1 ? STATUS_NDIS_LOW_POWER_STATE : InterlockedGet((LONG *)&ctx->PowerState) >= NdisDeviceStateD1 ? STATUS_NDIS_LOW_POWER_STATE :
@ -179,13 +179,11 @@ static NTSTATUS TunCheckForPause(_Inout_ TUN_CTX *ctx, _In_ LONG64 increment)
} }
_IRQL_requires_max_(DISPATCH_LEVEL) _IRQL_requires_max_(DISPATCH_LEVEL)
static NDIS_STATUS TunCompletePause(_Inout_ TUN_CTX *ctx, _In_ LONG64 decrement, _In_ BOOLEAN async_completion) static NDIS_STATUS TunCompletePause(_Inout_ TUN_CTX *ctx, _In_ BOOLEAN async_completion)
{ {
ASSERT(decrement <= InterlockedGet64(&ctx->ActiveTransactionCount)); ASSERT(InterlockedGet64(&ctx->ActiveTransactionCount) > 0);
if (!InterlockedSubtract64(&ctx->ActiveTransactionCount, decrement) && if (!InterlockedDecrement64(&ctx->ActiveTransactionCount) &&
InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_PAUSED, TUN_STATE_PAUSING) == TUN_STATE_PAUSING) { InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_PAUSED, TUN_STATE_PAUSING) == TUN_STATE_PAUSING) {
InterlockedExchange64(&ctx->Device.RefCount, 0);
TunIndicateStatus(ctx);
if (async_completion) if (async_completion)
NdisMPauseComplete(ctx->MiniportAdapterHandle); NdisMPauseComplete(ctx->MiniportAdapterHandle);
return NDIS_STATUS_SUCCESS; return NDIS_STATUS_SUCCESS;
@ -258,9 +256,7 @@ static IO_CSQ_COMPLETE_CANCELED_IRP TunCsqCompleteCanceledIrp;
_Use_decl_annotations_ _Use_decl_annotations_
static VOID TunCsqCompleteCanceledIrp(IO_CSQ *Csq, IRP *Irp) static VOID TunCsqCompleteCanceledIrp(IO_CSQ *Csq, IRP *Irp)
{ {
TUN_CTX *ctx = CONTAINING_RECORD(Csq, TUN_CTX, Device.ReadQueue.Csq);
TunCompleteRequest(Irp, 0, STATUS_CANCELLED); TunCompleteRequest(Irp, 0, STATUS_CANCELLED);
TunCompletePause(ctx, 1, TRUE);
} }
_IRQL_requires_same_ _IRQL_requires_same_
@ -335,7 +331,6 @@ retry:
if (!NT_SUCCESS(status)) { if (!NT_SUCCESS(status)) {
irp->IoStatus.Status = status; irp->IoStatus.Status = status;
IoCompleteRequest(irp, IO_NETWORK_INCREMENT); IoCompleteRequest(irp, IO_NETWORK_INCREMENT);
TunCompletePause(ctx, 1, TRUE);
goto retry; goto retry;
} }
@ -400,7 +395,7 @@ static BOOLEAN TunNBLRefDec(_Inout_ TUN_CTX *ctx, _Inout_ NET_BUFFER_LIST *nbl,
NET_BUFFER_LIST_NEXT_NBL(nbl) = NULL; NET_BUFFER_LIST_NEXT_NBL(nbl) = NULL;
NdisMSendNetBufferListsComplete(ctx->MiniportAdapterHandle, nbl, SendCompleteFlags); NdisMSendNetBufferListsComplete(ctx->MiniportAdapterHandle, nbl, SendCompleteFlags);
InterlockedSubtract(&ctx->PacketQueue.NumNbl, 1); InterlockedSubtract(&ctx->PacketQueue.NumNbl, 1);
TunCompletePause(ctx, 1, TRUE); TunCompletePause(ctx, TRUE);
return TRUE; return TRUE;
} }
return FALSE; return FALSE;
@ -575,7 +570,6 @@ static void TunQueueProcess(_Inout_ TUN_CTX *ctx)
} else { } else {
irp->IoStatus.Status = STATUS_SUCCESS; irp->IoStatus.Status = STATUS_SUCCESS;
IoCompleteRequest(irp, IO_NETWORK_INCREMENT); IoCompleteRequest(irp, IO_NETWORK_INCREMENT);
TunCompletePause(ctx, 1, TRUE);
irp = NULL; irp = NULL;
} }
@ -588,7 +582,7 @@ static DRIVER_DISPATCH TunDispatchCreate;
_Use_decl_annotations_ _Use_decl_annotations_
static NTSTATUS TunDispatchCreate(DEVICE_OBJECT *DeviceObject, IRP *Irp) static NTSTATUS TunDispatchCreate(DEVICE_OBJECT *DeviceObject, IRP *Irp)
{ {
NTSTATUS status = STATUS_UNSUCCESSFUL; NTSTATUS status = STATUS_SUCCESS;
TUN_CTX *ctx = TunGetContext(DeviceObject); TUN_CTX *ctx = TunGetContext(DeviceObject);
if (!ctx) { if (!ctx) {
@ -596,15 +590,10 @@ static NTSTATUS TunDispatchCreate(DEVICE_OBJECT *DeviceObject, IRP *Irp)
goto cleanup_complete_req; goto cleanup_complete_req;
} }
if (!NT_SUCCESS(status = TunCheckForPause(ctx, 1)))
goto cleanup_TunCompletePause;
ASSERT(InterlockedGet64(&ctx->Device.RefCount) < MAXLONG64); ASSERT(InterlockedGet64(&ctx->Device.RefCount) < MAXLONG64);
InterlockedIncrement64(&ctx->Device.RefCount); InterlockedIncrement64(&ctx->Device.RefCount);
TunIndicateStatus(ctx); TunIndicateStatus(ctx);
cleanup_TunCompletePause:
TunCompletePause(ctx, 1, TRUE);
cleanup_complete_req: cleanup_complete_req:
TunCompleteRequest(Irp, 0, status); TunCompleteRequest(Irp, 0, status);
return status; return status;
@ -614,7 +603,7 @@ static DRIVER_DISPATCH TunDispatchClose;
_Use_decl_annotations_ _Use_decl_annotations_
static NTSTATUS TunDispatchClose(DEVICE_OBJECT *DeviceObject, IRP *Irp) static NTSTATUS TunDispatchClose(DEVICE_OBJECT *DeviceObject, IRP *Irp)
{ {
NTSTATUS status = STATUS_UNSUCCESSFUL; NTSTATUS status = STATUS_SUCCESS;
TUN_CTX *ctx = TunGetContext(DeviceObject); TUN_CTX *ctx = TunGetContext(DeviceObject);
if (!ctx) { if (!ctx) {
@ -622,15 +611,10 @@ static NTSTATUS TunDispatchClose(DEVICE_OBJECT *DeviceObject, IRP *Irp)
goto cleanup_complete_req; goto cleanup_complete_req;
} }
if (!NT_SUCCESS(status = TunCheckForPause(ctx, 1)))
goto cleanup_TunCompletePause;
ASSERT(InterlockedGet64(&ctx->Device.RefCount) > 0); ASSERT(InterlockedGet64(&ctx->Device.RefCount) > 0);
InterlockedDecrement64(&ctx->Device.RefCount); InterlockedDecrement64(&ctx->Device.RefCount);
TunIndicateStatus(ctx); TunIndicateStatus(ctx);
cleanup_TunCompletePause:
TunCompletePause(ctx, 1, TRUE);
cleanup_complete_req: cleanup_complete_req:
TunCompleteRequest(Irp, 0, status); TunCompleteRequest(Irp, 0, status);
return status; return status;
@ -640,7 +624,7 @@ static DRIVER_DISPATCH TunDispatchRead;
_Use_decl_annotations_ _Use_decl_annotations_
static NTSTATUS TunDispatchRead(DEVICE_OBJECT *DeviceObject, IRP *Irp) static NTSTATUS TunDispatchRead(DEVICE_OBJECT *DeviceObject, IRP *Irp)
{ {
NTSTATUS status = STATUS_UNSUCCESSFUL; NTSTATUS status = STATUS_SUCCESS;
TUN_CTX *ctx = TunGetContext(DeviceObject); TUN_CTX *ctx = TunGetContext(DeviceObject);
if (!ctx) { if (!ctx) {
@ -648,24 +632,14 @@ static NTSTATUS TunDispatchRead(DEVICE_OBJECT *DeviceObject, IRP *Irp)
goto cleanup_complete_req; goto cleanup_complete_req;
} }
if (!NT_SUCCESS(status = TunCheckForPause(ctx, 1)))
goto cleanup_TunCompletePause;
Irp->IoStatus.Information = 0; Irp->IoStatus.Information = 0;
InterlockedIncrement64(&ctx->ActiveTransactionCount);
status = IoCsqInsertIrpEx(&ctx->Device.ReadQueue.Csq, Irp, NULL, TUN_CSQ_INSERT_TAIL); status = IoCsqInsertIrpEx(&ctx->Device.ReadQueue.Csq, Irp, NULL, TUN_CSQ_INSERT_TAIL);
if (!NT_SUCCESS(status)) { if (!NT_SUCCESS(status))
InterlockedDecrement64(&ctx->ActiveTransactionCount); goto cleanup_complete_req;
goto cleanup_TunCompletePause;
}
TunQueueProcess(ctx); TunQueueProcess(ctx);
TunCompletePause(ctx, 1, TRUE);
return STATUS_PENDING; return STATUS_PENDING;
cleanup_TunCompletePause:
TunCompletePause(ctx, 1, TRUE);
cleanup_complete_req: cleanup_complete_req:
TunCompleteRequest(Irp, 0, status); TunCompleteRequest(Irp, 0, status);
return status; return status;
@ -675,7 +649,7 @@ static DRIVER_DISPATCH TunDispatchWrite;
_Use_decl_annotations_ _Use_decl_annotations_
static NTSTATUS TunDispatchWrite(DEVICE_OBJECT *DeviceObject, IRP *Irp) static NTSTATUS TunDispatchWrite(DEVICE_OBJECT *DeviceObject, IRP *Irp)
{ {
NTSTATUS status = STATUS_UNSUCCESSFUL; NTSTATUS status = STATUS_SUCCESS;
ULONG_PTR information = 0; ULONG_PTR information = 0;
TUN_CTX *ctx = TunGetContext(DeviceObject); TUN_CTX *ctx = TunGetContext(DeviceObject);
@ -684,7 +658,7 @@ static NTSTATUS TunDispatchWrite(DEVICE_OBJECT *DeviceObject, IRP *Irp)
goto cleanup_complete_req; goto cleanup_complete_req;
} }
if (!NT_SUCCESS(status = TunCheckForPause(ctx, 1))) if (!NT_SUCCESS(status = TunCheckForPause(ctx)))
goto cleanup_TunCompletePause; goto cleanup_TunCompletePause;
UCHAR *buffer; UCHAR *buffer;
@ -796,7 +770,7 @@ static NTSTATUS TunDispatchWrite(DEVICE_OBJECT *DeviceObject, IRP *Irp)
information = b - buffer; information = b - buffer;
cleanup_TunCompletePause: cleanup_TunCompletePause:
TunCompletePause(ctx, 1, TRUE); TunCompletePause(ctx, TRUE);
cleanup_complete_req: cleanup_complete_req:
TunCompleteRequest(Irp, information, status); TunCompleteRequest(Irp, information, status);
return status; return status;
@ -806,7 +780,7 @@ static DRIVER_DISPATCH TunDispatchCleanup;
_Use_decl_annotations_ _Use_decl_annotations_
static NTSTATUS TunDispatchCleanup(DEVICE_OBJECT *DeviceObject, IRP *Irp) static NTSTATUS TunDispatchCleanup(DEVICE_OBJECT *DeviceObject, IRP *Irp)
{ {
NTSTATUS status = STATUS_UNSUCCESSFUL; NTSTATUS status = STATUS_SUCCESS;
TUN_CTX *ctx = TunGetContext(DeviceObject); TUN_CTX *ctx = TunGetContext(DeviceObject);
if (!ctx) { if (!ctx) {
@ -814,19 +788,11 @@ static NTSTATUS TunDispatchCleanup(DEVICE_OBJECT *DeviceObject, IRP *Irp)
goto cleanup_complete_req; goto cleanup_complete_req;
} }
LONG64 count = 1;
if (!NT_SUCCESS(status = TunCheckForPause(ctx, count)))
goto cleanup_TunCompletePause;
IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(Irp); IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(Irp);
IRP *pending_irp; IRP *pending_irp;
while ((pending_irp = IoCsqRemoveNextIrp(&ctx->Device.ReadQueue.Csq, stack->FileObject)) != NULL) { while ((pending_irp = IoCsqRemoveNextIrp(&ctx->Device.ReadQueue.Csq, stack->FileObject)) != NULL)
count++;
TunCompleteRequest(pending_irp, 0, STATUS_CANCELLED); TunCompleteRequest(pending_irp, 0, STATUS_CANCELLED);
}
cleanup_TunCompletePause:
TunCompletePause(ctx, count, TRUE);
cleanup_complete_req: cleanup_complete_req:
TunCompleteRequest(Irp, 0, status); TunCompleteRequest(Irp, 0, status);
return status; return status;
@ -846,9 +812,7 @@ static NDIS_STATUS TunPause(NDIS_HANDLE MiniportAdapterContext, PNDIS_MINIPORT_P
{ {
TUN_CTX *ctx = (TUN_CTX *)MiniportAdapterContext; TUN_CTX *ctx = (TUN_CTX *)MiniportAdapterContext;
LONG64 count = 1; InterlockedIncrement64(&ctx->ActiveTransactionCount);
InterlockedAdd64(&ctx->ActiveTransactionCount, count);
if (InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_PAUSING, TUN_STATE_RUNNING) != TUN_STATE_RUNNING) { if (InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_PAUSING, TUN_STATE_RUNNING) != TUN_STATE_RUNNING) {
InterlockedDecrement64(&ctx->ActiveTransactionCount); InterlockedDecrement64(&ctx->ActiveTransactionCount);
return NDIS_STATUS_FAILURE; return NDIS_STATUS_FAILURE;
@ -856,14 +820,7 @@ static NDIS_STATUS TunPause(NDIS_HANDLE MiniportAdapterContext, PNDIS_MINIPORT_P
TunQueueClear(ctx); TunQueueClear(ctx);
/* Cancel pending IRPs to unblock waiting clients. */ return TunCompletePause(ctx, FALSE);
IRP *pending_irp;
while ((pending_irp = IoCsqRemoveNextIrp(&ctx->Device.ReadQueue.Csq, NULL)) != NULL) {
count++;
TunCompleteRequest(pending_irp, 0, STATUS_CANCELLED);
}
return TunCompletePause(ctx, count, FALSE);
} }
static MINIPORT_RESTART TunRestart; static MINIPORT_RESTART TunRestart;
@ -874,9 +831,6 @@ static NDIS_STATUS TunRestart(NDIS_HANDLE MiniportAdapterContext, PNDIS_MINIPORT
if (InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_RESTARTING, TUN_STATE_PAUSED) != TUN_STATE_PAUSED) if (InterlockedCompareExchange((LONG *)&ctx->State, TUN_STATE_RESTARTING, TUN_STATE_PAUSED) != TUN_STATE_PAUSED)
return NDIS_STATUS_FAILURE; return NDIS_STATUS_FAILURE;
ASSERT(!InterlockedGet64(&ctx->Device.RefCount));
TunIndicateStatus(ctx);
InterlockedExchange((LONG *)&ctx->State, TUN_STATE_RUNNING); InterlockedExchange((LONG *)&ctx->State, TUN_STATE_RUNNING);
return NDIS_STATUS_SUCCESS; return NDIS_STATUS_SUCCESS;
} }
@ -1197,7 +1151,11 @@ static void TunHaltEx(NDIS_HANDLE MiniportAdapterContext, NDIS_HALT_ACTION HaltA
return; return;
ASSERT(!InterlockedGet64(&ctx->ActiveTransactionCount)); ASSERT(!InterlockedGet64(&ctx->ActiveTransactionCount));
ASSERT(!InterlockedGet64(&ctx->Device.RefCount));
/* Cancel pending IRPs to unblock waiting clients. */
IRP *pending_irp;
while ((pending_irp = IoCsqRemoveNextIrp(&ctx->Device.ReadQueue.Csq, NULL)) != NULL)
TunCompleteRequest(pending_irp, 0, STATUS_CANCELLED);
/* Reset adapter context in device object, as Windows keeps calling dispatch handlers even after NdisDeregisterDeviceEx(). */ /* Reset adapter context in device object, as Windows keeps calling dispatch handlers even after NdisDeregisterDeviceEx(). */
TUN_CTX * volatile * control_device_extension = TunGetContextPointer(ctx->Device.Object); TUN_CTX * volatile * control_device_extension = TunGetContextPointer(ctx->Device.Object);
@ -1404,7 +1362,7 @@ static void TunSendNetBufferLists(NDIS_HANDLE MiniportAdapterContext, NET_BUFFER
TUN_CTX *ctx = (TUN_CTX *)MiniportAdapterContext; TUN_CTX *ctx = (TUN_CTX *)MiniportAdapterContext;
NDIS_STATUS status; NDIS_STATUS status;
if (!NT_SUCCESS(status = TunCheckForPause(ctx, 1))) { if (!NT_SUCCESS(status = TunCheckForPause(ctx))) {
TunSetNBLStatus(NetBufferLists, status); TunSetNBLStatus(NetBufferLists, status);
NdisMSendNetBufferListsComplete(ctx->MiniportAdapterHandle, NetBufferLists, SendFlags & NDIS_SEND_FLAGS_DISPATCH_LEVEL ? NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL : 0); NdisMSendNetBufferListsComplete(ctx->MiniportAdapterHandle, NetBufferLists, SendFlags & NDIS_SEND_FLAGS_DISPATCH_LEVEL ? NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL : 0);
goto cleanup_TunCompletePause; goto cleanup_TunCompletePause;
@ -1414,7 +1372,7 @@ static void TunSendNetBufferLists(NDIS_HANDLE MiniportAdapterContext, NET_BUFFER
TunQueueProcess(ctx); TunQueueProcess(ctx);
cleanup_TunCompletePause: cleanup_TunCompletePause:
TunCompletePause(ctx, 1, TRUE); TunCompletePause(ctx, TRUE);
} }
DRIVER_INITIALIZE DriverEntry; DRIVER_INITIALIZE DriverEntry;