Commit Graph

14 Commits

Author SHA1 Message Date
Simon Rozman
928f21c573 driver: switch to MS-recommended memory alloc
Suggested-by: Static Driver Verifier
Signed-off-by: Simon Rozman <simon@rozman.si>
2021-07-13 14:33:43 +02:00
Simon Rozman
d41ac04565 driver: remove excessive media connection reporting on adapter init
The initial adapter state (including media connection) is provided by
the NDIS_MINIPORT_ADAPTER_GENERAL_ATTRIBUTES.
Additional NdisMIndicateStatusEx() call seems excessive.

Signed-off-by: Simon Rozman <simon@rozman.si>
2021-07-12 10:38:21 +02:00
Jason A. Donenfeld
d33732ab4b driver: hard code security descriptor bytes
This is compatible with old Windows. Generated by:

  #include <stdio.h>
  #include <windows.h>
  #include <sddl.h>

  int main(int argc, char *argv[])
  {
      PSECURITY_DESCRIPTOR sd;
      ULONG sd_len;

      if (!ConvertStringSecurityDescriptorToSecurityDescriptorA("O:SYD:P(A;;FA;;;SY)(A;;FA;;;BA)S:(ML;;NWNRNX;;;HI)", SDDL_REVISION_1, &sd, &sd_len))
          return 1;
      for (ULONG i = 0; i < sd_len; ++i)
          printf("0x%02x%s%s", ((unsigned char *)sd)[i], i == sd_len - 1 ? "" : ",", i == sd_len -1 || i % 8 == 7 ? "\n": " ");
      return 0;
  }

This can be easily checked from kernel space with this ugly snippet:

  UNICODE_STRING Func;
  RtlInitUnicodeString(&Func, L"SeConvertSecurityDescriptorToStringSecurityDescriptor");
  WCHAR *Str = NULL;
  ((NTSTATUS(NTAPI *)(PSECURITY_DESCRIPTOR, DWORD, DWORD, WCHAR **, DWORD *))MmGetSystemRoutineAddress(&Func))(
      TunDispatchSecurityDescriptor, 1, 0x14, &Str, NULL);
  DbgPrint("Did it work? %ls\n", Str);

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-06-25 16:18:03 +02:00
Jason A. Donenfeld
6154c73032 driver: build security descriptor from sddl
This is a bit easier to read.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-06-25 14:10:50 +02:00
Jason A. Donenfeld
b3bf490434 driver: allow admins but require high integrity label
Might be more reasonable.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-06-25 14:09:16 +02:00
Jason A. Donenfeld
273cbe2d14 driver: format
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-06-25 13:51:57 +02:00
Jason A. Donenfeld
6cf9ac71c3 driver: do not assume aligned addresses when allocating MDLs
IoAllocateMdl allocates a different size structure depending on the
bottom in-page bits of the address. By passing null, it assumes that the
address is aligned within the page, which it might not be. Fix this by
passing the eventual virtual address to the allocation function so that
the right amount is always allocated.

Reported-by: Oleksandr Muzychuk <oleksandr.muzychuk@apriorit.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-05-10 19:02:49 +02:00
Simon Rozman
f19945b3c6 driver: move init-only functions into INIT segment
Reference: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/writing-a-driverentry-routine
Signed-off-by: Simon Rozman <simon@rozman.si>
2021-05-10 11:24:44 +02:00
Simon Rozman
71e1ab6940 driver: fix memory leak on pre-Windows 7
Should NDIS version check fail the DriverEntry() bailed out without
releasing the TunDispatchSecurityDescriptor.

Signed-off-by: Simon Rozman <simon@rozman.si>
2021-05-10 11:24:44 +02:00
Jason A. Donenfeld
2628412d71 global: bump copyright
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2021-01-30 16:45:26 +01:00
Jason A. Donenfeld
b2374dadb6 driver: use IoAllocateMdl without being too clever
Windows 7 doesn't like our trick of sticking MDLs into the NBL context
area. So we do things more traditionally, by allocating an MDL with
IoAllocateMdl and freeing it with IoFreeMdl. This means that we have to
keep track of the MDL between allocation and free time, and we don't
have any more miniport reserved pointers left in the NBL. So instead we
walk the MdlChain field of the first NB, and free the one that has an
address living inside of the non-partial MDL.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2020-12-15 21:13:06 +01:00
Jason A. Donenfeld
1269f86c76 driver: use partial MDL for slicing ring, rather than NB's DataOffset
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>
2020-12-13 18:31:44 +01:00
Jason A. Donenfeld
87ef399d1c driver: do not allow compiler to reload PacketSize
In theory, the compiler could reload PacketSize after the bounds check
but before it's passed to NdisAllocateNetBufferAndNetBufferList. In
practice, it's not actually doing that, but better safe than sorry.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
2020-11-09 22:23:26 +01:00
Simon Rozman
a00c8ca685 driver: move to subfolder
Signed-off-by: Simon Rozman <simon@rozman.si>
2020-11-06 07:29:47 +01:00