aboutsummaryrefslogtreecommitdiffstats
path: root/driver (follow)
Commit message (Collapse)AuthorAgeFilesLines
* driver: fix missed-wakeup race in ring buffer Alertable signalingHEADmasterSimon Rozman2026-03-181-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add MemoryBarrier() between store-load pairs in the Dekker-style synchronization used by the Receive ring's Alertable/Tail protocol. On x86-64, WriteRelease/ReadAcquire only prevent compiler reordering and provide acquire/release semantics, but do not emit MFENCE — the only instruction that prevents store-load reordering across cores. Without a full barrier, both the userspace producer and the kernel consumer can simultaneously read stale values: Userspace: STORE(Tail) ... LOAD(Alertable) -> sees FALSE (stale) Driver: STORE(Alertable=TRUE) ... LOAD(Tail) -> sees old tail The driver then enters KeWaitForMultipleObjects with no pending SetEvent, sleeping until a TCP retransmission (typically 4-5s later) re-triggers the send path and wins the race. The fix adds MemoryBarrier() (MFENCE on x86) on both sides: - api/session.c WintunSendPacket: between WriteULongRelease(Tail) and ReadAcquire(Alertable) - driver/twintun.c TunProcessReceiveData: between WriteRelease(Alertable, TRUE) and ReadULongAcquire(Tail) This guarantees that at least one side always observes the other's store, preventing the missed wakeup while preserving the Alertable optimization that avoids unnecessary SetEvent syscalls. Reported-by: Alexey Lapuka <alexey@twingate.com> Reference: https://lists.zx2c4.com/pipermail/wireguard/2026-February/009523.html Signed-off-by: Simon Rozman <simon.rozman@amebis.si>
* driver: fix race condition at Ring->HeadSimon Rozman2026-02-201-6/+5
| | | | | | | | | | | | | | | | | | | | | | | | This PR fixes a critical race condition in the Wintun driver that causes ring buffer overruns when multiple UDP senders operate in parallel. The issue occurred because multiple threads could read the same Ring->Head value without synchronization, leading to concurrent modifications that corrupted the ring buffer and resulted in ERROR_INVALID_DATA errors. In order to prevent buffer overrun (which was observed while sending multiple high throughput UDP streams from different threads) I move the driver spinlock to protect Ring buffer Head. I observed that the Ring->Head was taken and manipulated later on with just a `ReadULongAcquire` which isn't OK when 2 are trying to manipulate it later on based on the same received value. A fix was provided in 4fc590853b8281552631b79dacda484a5782f3bf, but it didn't solve the issue. Reported-by: odedkatz <katz.oded@gmail.com> Reference: https://lists.zx2c4.com/pipermail/wireguard/2026-February/009489.html Reference: https://lists.zx2c4.com/pipermail/wireguard/2026-February/009510.html Signed-off-by: Simon Rozman <simon.rozman@amebis.si>
* driver: explicitly terminate NBLSimon Rozman2026-02-201-3/+4
| | | | | | | | | | | The ActiveNbls list uses MINIPORT_RESERVED[1] as the next pointer, but never NULL-terminates the tail entry. It was never observed the allocated NBL would contain a non-zero MiniportReserved array, but this being unspecified in the official documentation, let's stay out of "unspecified" area. Reference: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/nbl/ns-nbl-net_buffer_list Signed-off-by: Simon Rozman <simon.rozman@amebis.si>
* driver: introduce NET_BUFFER_LIST_OFFSET helperSimon Rozman2026-02-091-4/+6
| | | | | | | | | Since NET_BUFFER_LIST_MINIPORT_RESERVED[1] is assigned our next NBL pointer and we have a macro for it (NET_BUFFER_LIST_NEXT_NBL_EX), let's have a macro for NET_BUFFER_LIST_MINIPORT_RESERVED[0], we assigned NBL offset and flags to, too. Signed-off-by: Simon Rozman <simon.rozman@amebis.si>
* driver: fix spelling mistakes in commentsJason A. Donenfeld2022-05-231-2/+2
| | | | | Suggested-by: Dimitri Papadopoulos <dimitri.papadopoulos@cea.fr> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: address Code Analysis warningSimon Rozman2021-10-171-0/+1
| | | | | | | In this case, we're referencing objects by handle for closing user space handles. Should be UserMode, not Irp->RequestorMode as C28126 suggests. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: allow ring registration from kernelSimon Rozman2021-10-171-4/+4
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: address Code Analysis warningsSimon Rozman2021-10-171-1/+2
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: automatically close long-lived handleJason A. Donenfeld2021-10-121-70/+77
| | | | | | | | | | | | | There's only one handle that's likely to be open in a long lived way: the tun registration handle. So we can force that closed automatically when the device is about to close, if it's been improperly left open. Other handles will indeed hold up closing, but if those exist, they're a sign of a larger bug elsewhere that should be addressed. On the other hand, tun registration handles might legitimately be open during driver upgrades. This also saves us the trouble of dereferencing a freed FileObject as in the general case. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: rewrite based on SwDeviceJason A. Donenfeld2021-10-121-1/+4
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: inf: remove LoadOrderGroupJason A. Donenfeld2021-09-241-1/+0
| | | | | | It's useless for PnP drivers. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: allow userspace to write garbageJason A. Donenfeld2021-09-161-2/+3
| | | | | | | | Not discouraging userspace from skipping checking IP packets seems like a bad thing, but they skip it anyway, so at least avoid the DoS due to API misuse. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: Replace depreciated ExAllocatePoolWithTagSimon Rozman2021-09-131-1/+1
| | | | | | | | CodeQL with Windows-Driver-Developer-Supplemental-Tools suggests the ExAllocatePoolWithTag() should no longer be used. The Static Tools Logo Test in HLK spots this in the DVL log and fails. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: mark as network driver rather than system driverJason A. Donenfeld2021-08-071-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: remove useless defines from resourceJason A. Donenfeld2021-07-301-3/+0
| | | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Signed-off-by: Simon Rozman <simon@rozman.si>
* vs: move shared configuration to wintun.props and upgradeSimon Rozman2021-07-281-83/+5
| | | | | | | Remember to rename wintun.vcxproj.user file in your local working folder to wintun.props.user manually. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: workaround SDV failure with code analysisSimon Rozman2021-07-271-5/+6
| | | | | | | | | | | | | | SDV is using own CL.EXE which returns error code 2 when code analysis is turned on. However, we need code analysis results for DVL. While we could use a new "ReleaseSDV" configuration, we don't really require limited code analysis in Release builds, as long as we address all full code analysis warnings in Debug builds. To make DVL happier, an intermediate Release build was injected with code analysis turned on. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: switch to MS-recommended memory allocSimon Rozman2021-07-132-3/+5
| | | | | Suggested-by: Static Driver Verifier Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: cleanup project fileSimon Rozman2021-07-121-155/+132
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: remove excessive media connection reporting on adapter initSimon Rozman2021-07-121-4/+1
| | | | | | | | 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>
* api: use SuggestedInstanceId instead of NetSetupAnticipatedInstanceIdJason A. Donenfeld2021-07-091-131/+155
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All was well with NetSetupAnticipatedInstanceId, until a bug crept into recent Windows builds that caused old GUIDs not to be properly removed, resulting in subsequent adapter creations to fail, because NetSetup AnticipatedInstanceId considers it fatal when the target GUID already exists, even if in diminished form. The initial solution was to detect cruft, and then steal a TrustedInstaller token and sleuth around the registry cleaning things up. The horror! Uncomfortable with this, I reopened IDA and had a look around with fresh eyes, three years after the original discovery of NetSetupAnticipated InstanceId. There, I found some interesting behavior in NetSetupSvcDeviceManager::InstallNetworkInterfaces, which amounts to something like: if (IsSet("RetiredNetCfgInstanceId") { if (IsSet("NetSetupAnticipatedInstanceId") DeleteAdapter(GetValue("RetiredNetCfgInstanceId")); else Set("NetSetupAnticipatedInstanceId", GetValue("RetiredNetCfgInstanceId")); Delete("RetiredNetCfgInstanceId"); } CreateAdapter = TRUE; if (IsSet("NetSetupAnticipatedInstanceId")) { Guid = GetValue("NetSetupAnticipatedInstanceId"); if (AdapterAlreadyExists(Guid)) CreateAdapter = FALSE; else SetGuidOfNewAdapter(Guid); Delete("NetSetupAnticipatedInstanceId"); } else if (IsSet("SuggestedInstanceId")) { Guid = GetValue("SuggestedInstanceId"); if (!AdapterAlreadyExists(Guid)) SetGuidOfNewAdapter(Guid); Delete("SuggestedInstanceId"); } Thus, one appealing strategy would be to set both NetSetupAnticipated InstanceId and RetiredInstanceId to the same value, and let the service handle deleting the old one for us before creating the new one. However, the cleanup of the old adapter winds up being quasi- asynchronous, and thus we still wind up in the CreateAdapter = FALSE case. So, the remaining strategy is to simply use SuggestedInstanceId instead. This has the behavior that if there's an adapter already in use, it'll use a new random GUID. The result is that adapter creation won't fail. That's not great, but the docs have always made it clear that "requested" is a best-effort sort of thing. Plus, hopefully the creation of the new adapter will help nudge the bug a bit and cleanup the old cruft. In some ways, transitioning from our old strategy of "cudgel the registry until we get the GUID we want" to "ask politely and accept no for an answer" is a disappointing regression in functionality. But it also means we don't need to keep crazy token stealing code around, or fish around in the registry dangerously. This probably also increases the likelihood that an adapter will be created during edge cases, which means fewer errors for users, which could be a good thing. On the downside, we have the perpetual tensions caused by a system that now "fails open" instead of "fails closed". But so it goes in Windows land. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: hard code security descriptor bytesJason A. Donenfeld2021-06-251-18/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* driver: build security descriptor from sddlJason A. Donenfeld2021-06-253-100/+10
| | | | | | This is a bit easier to read. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: allow admins but require high integrity labelJason A. Donenfeld2021-06-253-8/+70
| | | | | | Might be more reasonable. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: specify pnplockdown in infJason A. Donenfeld2021-06-251-0/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: formatJason A. Donenfeld2021-06-251-5/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: do not assume aligned addresses when allocating MDLsJason A. Donenfeld2021-05-101-2/+4
| | | | | | | | | | | 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>
* driver: move init-only functions into INIT segmentSimon Rozman2021-05-101-0/+9
| | | | | Reference: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/writing-a-driverentry-routine Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: fix memory leak on pre-Windows 7Simon Rozman2021-05-101-3/+2
| | | | | | | Should NDIS version check fail the DriverEntry() bailed out without releasing the TunDispatchSecurityDescriptor. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: cleanup unused DBG defineSimon Rozman2021-05-101-6/+0
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* Fix © in resourcesSimon Rozman2021-03-191-0/+2
| | | | | | | | The \xa9 is © on Windows-125x code pages. When Wintun was compiled on a Windows computer using UTF-8 as default code page (for "non-Unicode" programs), the Copyright notice in resources was wrong. Signed-off-by: Simon Rozman <simon@rozman.si>
* project: add support for intermediate versioningSimon Rozman2021-03-191-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | While the Wintun driver is typically released only at <major>.<minor> milestones, the wintun.dll did see some intermediate releases. To make MSI logic correctly decide to upgrade local wintun.dll file, the version inscribed in wintun.dll file resources should increment in those intermediate releases. MSI ignores file timestamps. One could use REINSTALLMODE=emus Installer property to force copying wintun.dll when its version doesn't change. But REINSTALLMODE applies to all files in the MSI session and would be an additional requirement when authoring MSI packages with wintun.dll. Bumping only the final ZIP filename version is not sufficient. Therefore, a <major>.<minor> or <major>.<minor>.<build> versioning is introduced. Furthermore, we no longer distinguish between WintunVersion and WintunVersionStr. All our releases used strictly numeric <major>.<minor> notation, making WintunVersion and WintunVersionStr always the same. When the driver didn't change, just bump the version in wintun.proj and run `msbuild wintun.proj /t:Zip` to rebuild the wintun.dll and make the new ZIP file. Signed-off-by: Simon Rozman <simon@rozman.si>
* global: bump copyrightJason A. Donenfeld2021-01-304-5/+5
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: use IoAllocateMdl without being too cleverJason A. Donenfeld2020-12-151-8/+17
| | | | | | | | | | | | 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>
* driver: use partial MDL for slicing ring, rather than NB's DataOffsetJason A. Donenfeld2020-12-131-3/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* driver: use localtime in inf2catStefan Rinkes2020-12-021-0/+1
| | | | | | | Otherwise the build fails at odd hours of the day. Signed-off-by: Stefan Rinkes <stefan.rinkes@gmail.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: do not allow compiler to reload PacketSizeJason A. Donenfeld2020-11-091-1/+1
| | | | | | | | 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>
* driver: move to subfolderSimon Rozman2020-11-066-0/+1831
Signed-off-by: Simon Rozman <simon@rozman.si>
Copyright © 1996 – 2026 Jason A. Donenfeld. All Rights Reverse Engineered.