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

driver: use partial MDL for slicing ring, rather than NB's DataOffset

Browse Source 
Providing the DataOffset member of the NBL allocation function or
setting that member in the NB header indicates to NDIS not only that the
data starts at that offset, but that there's that amount of space
*available for it to use as it wants* before that offset. This meant
that NDIS was allowed to scribble data before the packet.

This was bounded by the size of the ring, so there was never any risk of
memory corruption, and since the ring is shared by userspace as well as
the rest of the kernel, we've always taken care of reading from it
closely, checking all values, and erroring out on corruption of the
ring. So, if NDIS wrote before the first packet, this would wind up
corrupting the RingTail and Alertable fields of the ring. The receiver
thread would then notice this, error out, and set the RingHead to
MAXULONG on its way out the door, so that userspace can detect it. And
indeed wintun.dll then started returning EOF from its write function.

Mostly this was not an issue, because we're not expecting for data to be
pushed on the head of a packet on ingress. But WSL2's Hyper-V driver is
actually pushing an ethernet header onto the front of the packet before
passing it off to Linux. Practically speaking, this manifested itself in
the RingTail and Alertable fields having Linux's MAC address! And then
the adapter would be EOF'd. This was reported as happening after WSL2
sends the *first* packet, but not others, which makes sense, because it
has to be at the beginning in order to corrupt those fields.

This fixes the problem by simply using a new MDL for the span we want,
instead of using the misunderstood DataOffset field. In order to not
need to keep track of memory allocations, we allocate the MDL as part of
the NBL's context area. And in order to avoid additional mappings, we
use IoBuildPartialMdl, which returns an MDL_PARTIAL, which does not have
an additional mapping that needs to be freed or unmapped.

After making this change, WSL2 no longer appears to halt the adapter,
and all works well.

Fixes: be8d2cb ("Avoid allocating second MDL")
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This commit is contained in:
Jason A. Donenfeld 2020-12-12 15:20:11 +01:00
parent dd33757718
commit 1269f86c76
1 changed files with 12 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
driver/wintun.c
View File

@ -507,11 +507,20 @@ TunProcessReceiveData(_Inout_ TUN_CTX *Ctx)
break;
RingHead = TUN_RING_WRAP(RingHead + AlignedPacketSize, RingCapacity);
NET_BUFFER_LIST *Nbl = NdisAllocateNetBufferAndNetBufferList(
Ctx->NblPool, 0, 0, Ctx->Device.Receive.Mdl, (ULONG)(Packet->Data - (UCHAR *)Ring), PacketSize);
MDL *Mdl;
NET_BUFFER_LIST *Nbl = NdisAllocateNetBufferAndNetBufferList(Ctx->NblPool, sizeof(*Mdl), 0, NULL, 0, 0);
if (!Nbl)
goto skipNbl;
Mdl = (MDL *)NET_BUFFER_LIST_CONTEXT_DATA_START(Nbl);
IoBuildPartialMdl(
Ctx->Device.Receive.Mdl,
Mdl,
(UCHAR *)MmGetMdlVirtualAddress(Ctx->Device.Receive.Mdl) + (ULONG)(Packet->Data - (UCHAR *)Ring),
PacketSize);
NET_BUFFER *Nb = NET_BUFFER_LIST_FIRST_NB(Nbl);
NET_BUFFER_FIRST_MDL(Nb) = NET_BUFFER_CURRENT_MDL(Nb) = Mdl;
NET_BUFFER_DATA_LENGTH(Nb) = PacketSize;
NET_BUFFER_DATA_OFFSET(Nb) = NET_BUFFER_CURRENT_MDL_OFFSET(Nb) = 0;
Nbl->SourceHandle = Ctx->MiniportAdapterHandle;
NdisSetNblFlag(Nbl, NblFlags);
NET_BUFFER_LIST_INFO(Nbl, NetBufferListFrameType) = (PVOID)NblProto;