Skip to content

Commit

Permalink
Fix BSOD on ARM64
Browse files Browse the repository at this point in the history 
Commit "eb764efe1" (rxpath: fix broken TCP connections on Windows Server)
fixed TCP performance on Windows Server 2019 / 2022, but causes BSOD in
mrxsmb.sys driver when running on ARM64 (reproduced on Windows 10 and 11).

SYSTEM_THREAD_EXCEPTION_NOT_HANDLED_M (1000007e)

5: kd> kn
 # Child-SP          RetAddr               Call Site
00 fffff302`14353430 fffff803`12bc4a20     nt!KeBugCheck2+0x234
01 fffff302`14353a10 fffff803`1280e538     nt!PspSystemThreadStartup$filt$0+0x58
02 fffff302`14353a20 fffff803`12a67dcc     nt!_C_ExecuteExceptionFilter+0x38
03 fffff302`14353a80 fffff803`1280cf34     nt!_C_specific_handler+0xcc
04 fffff302`14353ae0 fffff803`12905d48     nt!RtlpExecuteHandlerForException+0x14
05 fffff302`14353b00 fffff803`12877fd4     nt!RtlDispatchException+0x2e8
06 fffff302`14354160 fffff803`1287856c     nt!KiDispatchException+0x3f4
07 fffff302`14354650 fffff803`128c07d0     nt!KiDispatchExceptionOnExceptionStack+0xc4
08 fffff302`14354680 fffff803`12803c00     nt!KiSynchronousException+0xd0
09 fffff302`14354770 fffff803`1280285c     nt!KzSynchronousException+0x24
0a fffff302`143547d0 fffff803`17009adc     nt!KiArm64ExceptionVectors+0x5c
0b fffff302`14354b40 fffff803`170e1ff8     ndis!ndisValidOid+0x24
0c fffff302`14354b40 fffff803`1700957c     ndis!ndisMiniportOidIoctl+0x128
0d fffff302`14354cc0 fffff803`12856160     ndis!ndisDeviceControlHandler+0x17c
0e fffff302`14354d50 fffff803`12d3ce38     nt!IofCallDriver+0x30
0f fffff302`14354d80 fffff803`12d3e390     nt!IopSynchronousServiceTail+0x170
10 fffff302`14354e10 fffff803`12d49d3c     nt!IopXxxControlFile+0x658
11 fffff302`14355070 fffff803`1280c460     nt!NtDeviceIoControlFile+0x2c
12 fffff302`143550a0 fffff803`1280bf60     nt!KiSystemServiceCopyEnd+0x38
13 fffff302`14355110 fffff802`d88d7248     nt!KiServiceInternal+0x60
14 fffff302`14355470 fffff802`d88d6f28     mrxsmb!MRxSmbQueryLbfoTeamCapability+0x1d8
15 fffff302`143555c0 fffff803`17175754     mrxsmb!MRxSmbIPv4AddressChangeHandler+0x108
16 fffff302`143558c0 fffff803`17382228     NETIO!NsiParameterChange+0x244
17 fffff302`14355970 fffff803`17335564     tcpip!IppNotifyAddressChangeAtPassive+0x2d8
18 fffff302`14355a90 fffff803`1716f63c     tcpip!IppCompartmentNotificationWorker+0x74
19 fffff302`14355ac0 fffff803`1285a56c     NETIO!NetiopIoWorkItemRoutine+0x7c
1a fffff302`14355b20 fffff803`12980604     nt!IopProcessWorkItem+0xec
1b fffff302`14355b80 fffff803`128fa300     nt!ExpWorkerThread+0x114
1c fffff302`14355d50 fffff803`1280be4c     nt!PspSystemThreadStartup+0x50
1d fffff302`14355d90 00000000`00000000     nt!KiStartSystemThread+0x24

There are no tap-windows6 calls in this stack trace, but certain code change
in commit "eb764efe1" causes mrxsmb/ndis to bugcheck.

Note that this does _not_ happen on x64.

The problematic change seems to be passing NDIS_RECEIVE_FLAGS_RESOURCES
to NdisMIndicateReceiveNetBufferLists() in IndicateReceivePacket().
This code deals with DHCP / IPv6 ND. The bug check doesn't happen if
this flag is omitted in IndicateReceivePacket().

The reason why that flag was introduced is to process write requests
in place and not to pend them, which causes performance degradation
(see longer explanation in commit eb764ef.

However, IndicateReceivePacket() mostly deals with DHCP packets, which
are generated within the driver and are quire rare comparison
to normal data channel traffic, so partially reverting that commit
(removing NDIS_RECEIVE_FLAGS_RESOURCES and bringing back handling
"in-flight" packets) should not affect performance.

The root cause of bugcheck remains unknown since NDIS and mrxsmb are closed-
source and to my understanding tap-windows6 does nothing wrong in regards
to using NDIS_RECEIVE_FLAGS_RESOURCES flag and this is ARM64 specific issue.
In any case, I think that this fix/workaround is good enough.

Signed-off-by: Lev Stipakov <[email protected]>
  • Loading branch information
@lstipakov
lstipakov committed Nov 18, 2022
1 parent c3e61e8 commit 538d330
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 8 deletions.
75 changes: 75 additions & 0 deletions src/adapter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ tapAdapterContextAllocate(
// NBL pool for making TAP receive indications.
NdisZeroMemory(&nblPoolParameters, sizeof(NET_BUFFER_LIST_POOL_PARAMETERS));

// Initialize event used to determine when all receive NBLs have been returned.
NdisInitializeEvent(&adapter->ReceiveNblInFlightCountZeroEvent);

// Add initial reference. Normally removed in AdapterHalt.
adapter->RefCount = 1;

Expand Down Expand Up @@ -954,6 +957,63 @@ Return Value:
DEBUGP (("[TAP] <-- AdapterHalt\n"));
}

VOID
tapWaitForReceiveNblInFlightCountZeroEvent(
__in PTAP_ADAPTER_CONTEXT Adapter
)
{
LONG nblCount;

//
// Wait until higher-level protocol has returned all NBLs
// to the driver.
//

// Add one NBL "bias" to insure allow event to be reset safely.
nblCount = NdisInterlockedIncrement(&Adapter->ReceiveNblInFlightCount);
ASSERT(nblCount > 0);
NdisResetEvent(&Adapter->ReceiveNblInFlightCountZeroEvent);

//
// Now remove the bias and wait for the ReceiveNblInFlightCountZeroEvent
// if the count returned is not zero.
//
nblCount = NdisInterlockedDecrement(&Adapter->ReceiveNblInFlightCount);
ASSERT(nblCount >= 0);

if (nblCount)
{
LARGE_INTEGER startTime, currentTime;

NdisGetSystemUpTimeEx(&startTime);

for (;;)
{
BOOLEAN waitResult = NdisWaitEvent(
&Adapter->ReceiveNblInFlightCountZeroEvent,
TAP_WAIT_POLL_LOOP_TIMEOUT
);

NdisGetSystemUpTimeEx(&currentTime);

if (waitResult)
{
break;
}

DEBUGP(("[%s] Waiting for %d in-flight receive NBLs to be returned.\n",
MINIPORT_INSTANCE_ID(Adapter),
Adapter->ReceiveNblInFlightCount
));
}

DEBUGP(("[%s] Waited %d ms for all in-flight NBLs to be returned.\n",
MINIPORT_INSTANCE_ID(Adapter),
(currentTime.LowPart - startTime.LowPart)
));
}
}

NDIS_STATUS
AdapterPause(
__in NDIS_HANDLE MiniportAdapterContext,
Expand Down Expand Up @@ -1016,6 +1076,21 @@ Return Value:
adapter->Locked.AdapterState = MiniportPausingState;
tapAdapterReleaseLock(adapter,FALSE);

//
// Stop the flow of network data through the receive path
// ------------------------------------------------------
// In the Pausing and Paused state tapAdapterSendAndReceiveReady
// will prevent new calls to NdisMIndicateReceiveNetBufferLists
// to indicate additional receive NBLs to the host.
//
// However, there may be some in-flight NBLs owned by the driver
// that have been indicated to the host but have not yet been
// returned.
//
// Wait here for all in-flight receive indications to be returned.
//
tapWaitForReceiveNblInFlightCountZeroEvent(adapter);

//
// Stop the flow of network data through the send path
// ---------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions src/adapter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ typedef struct _TAP_ADAPTER_CONTEXT
// NBL pool for making TAP receive indications.
NDIS_HANDLE ReceiveNblPool;

volatile LONG ReceiveNblInFlightCount;
#define TAP_WAIT_POLL_LOOP_TIMEOUT 3000 // 3 seconds
NDIS_EVENT ReceiveNblInFlightCountZeroEvent;

// Info for point-to-point mode
BOOLEAN m_tun;
IPADDR m_localIP;
Expand Down
52 changes: 44 additions & 8 deletions src/rxpath.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
VOID
tapFreeReceiveNetBufferList(
__in PTAP_ADAPTER_CONTEXT Adapter,
__in PNET_BUFFER_LIST NetBufferList // Only one NB here...
__in PNET_BUFFER_LIST NetBufferList, // Only one NB here...
__in BOOLEAN DecrementInflight
)
{
ULONG frameType, netBufferCount, byteCount;
Expand Down Expand Up @@ -130,6 +131,17 @@ tapFreeReceiveNetBufferList(
NdisFreeMdl(mdl);
}

if (DecrementInflight)
{
// Decrement in-flight receive NBL count.
LONG nblCount = NdisInterlockedDecrement(&Adapter->ReceiveNblInFlightCount);
ASSERT(nblCount >= 0);
if (0 == nblCount)
{
NdisSetEvent(&Adapter->ReceiveNblInFlightCountZeroEvent);
}
}

// Free the NBL
NdisFreeNetBufferList(NetBufferList);
}
Expand Down Expand Up @@ -224,7 +236,7 @@ IndicateReceivePacket(

if(netBufferList != NULL)
{
ULONG receiveFlags = NDIS_RECEIVE_FLAGS_RESOURCES;
ULONG receiveFlags = 0;

NET_BUFFER_LIST_NEXT_NBL(netBufferList) = NULL; // Only one NBL

Expand All @@ -240,6 +252,10 @@ IndicateReceivePacket(
netBufferList->MiniportReserved[0] = NULL;
netBufferList->MiniportReserved[1] = NULL;

// Increment in-flight receive NBL count.
LONG nblCount = NdisInterlockedIncrement(&Adapter->ReceiveNblInFlightCount);
ASSERT(nblCount > 0);

netBufferList->SourceHandle = Adapter->MiniportAdapterHandle;

//
Expand All @@ -256,8 +272,6 @@ IndicateReceivePacket(
receiveFlags
);

tapFreeReceiveNetBufferList(Adapter->MiniportAdapterHandle, netBufferList);

return;
}
else
Expand Down Expand Up @@ -292,11 +306,33 @@ AdapterReturnNetBufferLists(
__in NDIS_HANDLE MiniportAdapterContext,
__in PNET_BUFFER_LIST NetBufferLists,
__in ULONG ReturnFlags
)
)
{
PTAP_ADAPTER_CONTEXT adapter = (PTAP_ADAPTER_CONTEXT )MiniportAdapterContext;
PTAP_ADAPTER_CONTEXT adapter = (PTAP_ADAPTER_CONTEXT)MiniportAdapterContext;
PNET_BUFFER_LIST currentNbl;

DEBUGP(("[%s] Unexpected AdapterReturnNetBufferLists() call\n", MINIPORT_INSTANCE_ID(adapter)));
UNREFERENCED_PARAMETER(ReturnFlags);

//
// Process each NBL individually
//
currentNbl = NetBufferLists;
while (currentNbl)
{
PNET_BUFFER_LIST nextNbl;

nextNbl = NET_BUFFER_LIST_NEXT_NBL(currentNbl);
NET_BUFFER_LIST_NEXT_NBL(currentNbl) = NULL;

// Complete write IRP and free NBL and associated resources.
tapFreeReceiveNetBufferList(
adapter,
currentNbl,
TRUE);

// Move to next NBL
currentNbl = nextNbl;
}
}

static PVOID
Expand Down Expand Up @@ -527,7 +563,7 @@ TapSharedSendPacket(
NDIS_RECEIVE_FLAGS_RESOURCES // ReceiveFlags
);

tapFreeReceiveNetBufferList(Adapter->MiniportAdapterHandle, netBufferList);
tapFreeReceiveNetBufferList(Adapter->MiniportAdapterHandle, netBufferList, FALSE);

return STATUS_SUCCESS;
}
Expand Down

0 comments on commit 538d330

Please sign in to comment.