TunCompleteRequest() no longer sets Information field in IRP and allows
to specify custom priority boost. This makes it suitable replacement for
all "set status; complete request; release remove lock"-tuples
throughout the code.
Functional changes in this patch:
- We no longer reset Information field to 0 for canceled IRPs. In other
words: ReadFile() of a canceled IRP will get the number of bytes read
before request was canceled in the lpNumberOfBytesRead, instead of
always 0.
- After write is complete, we boost user thread priority by +2
(IO_NETWORK_INCREMENT).
Signed-off-by: Simon Rozman <simon@rozman.si>
The NDIS reason why TunCheckForPause() reported the adapter unavailable,
might not make much sense to a client. In case adapter is paused or in
low power state, the STATUS_CANCELLED (ERROR_OPERATION_ABORTED) is
returned.
Exchange buffer size overflow - total TUN_EXCH_MAX_IP_PACKET_SIZE or
individual packet - rejects entire exchange buffer now.
Exchange buffers containing non-IPv4 or non-IPv6 packets are now
rejected as a whole.
Allocation errors while preparing NBLs from the exchange buffer are now
considered fatal.
Ensure write buffer has at least sizeof(TUN_PACKET) left, or reject
entire exchange buffer.
Signed-off-by: Simon Rozman <simon@rozman.si>
Commentary from Jason:
Problem statement:
We call IoCompleteRequest(Irp) immediately after
NdisMIndicateReceiveNetBufferLists, which frees Irp->MdlAddress.
Since we've just given the same memory to
NdisMIndicateReceiveNetBufferLists (in a different MDL), we wind up
freeing the memory before NDIS finishes processing them.
Fix possibility 1:
Move IoCompleteRequest(Irp) to TunReturnNetBufferLists. This requires
reference counting how many NBLs are currently in flight that are
using an IRP. When that drops to zero, we can call IoCompleteRequest
(Irp).
Problem:
This means we have to block future wireguard-go Writes until *all*
NBLs have completed processing in the networking stack. Is that safe
to do? Will that introduce latency? Can userspace processes sabotage
it by refusing to read from a TCP socket buffer? We don't know enough
about how NdisMIndicateReceiveNetBufferLists works to assess its
characteristics here.
Fix possibility 2:
Use NDIS_RECEIVE_FLAGS_RESOURCES, so that
NdisMIndicateReceiveNetBufferLists makes a copy, and then we'll simply
free everything immediately after. This is slow, and it could
potentially lead to wireguard-go making the kernel allocate lots of
memory in the case that NdisAllocateNetBufferAndNetBufferList doesn't
ratelimit its creation in the same way Linux's skb_alloc does.
However, it does make the lifetime of Irps shorter, which is easier to
analyze, and it might lead to better latency, since we don't need to
wait until userspace sends its next packets, so long as Ndis'
ingestion queue doesn't become too large.
This commit switches from (2) to (1).
Signed-off-by: Simon Rozman <simon@rozman.si>
When adapter is in disconnected state, NDIS does not send it any NBLs.
After transition to disconnected state it should return all pending NBLs
back to NDIS, otherwise a deadlock occurs on pause attempt later.
Likewise when the adapter is in low-power state.
Signed-off-by: Simon Rozman <simon@rozman.si>
No need for breaking every XML tag with attributes into lines - besides,
we're imposing 2-space indentation on .proj files making lines even
shorter and indentation combined with excessive line breaking harder to
follow visually.
Signed-off-by: Simon Rozman <simon@rozman.si>
This is temporary until we figure out how to use WiX's Difx to install
driver without leaving staged driver files behind.
Signed-off-by: Simon Rozman <simon@rozman.si>
This allows common version and other configuration in wintun.props.
- MSM packaging migrated to WiX MSBuild project and extended to support
digital signing
- Building supports single Platform|Configuration per run - again :(
- wintun.vcxproj cleanup
Signed-off-by: Simon Rozman <simon@rozman.si>
Revision (third) and build (fourth) fields were also dropped:
- NDIS_MINIPORT_DRIVER_CHARACTERISTICS supports only a.b
- MSI checks only a.b.c
- GPO compares only a.b when checking if the new MSI is an update
Signed-off-by: Simon Rozman <simon@rozman.si>
wintun.inf and wintun.cat have no version resource. WiX linker cannot
determine their versions and the MSM will use file sizes and hashes for
update logic. By setting explicit file version and language, the version
will always be used to determine which version is newer on updates.
Signed-off-by: Simon Rozman <simon@rozman.si>
Adopt PROCESSOR_ARCHITECTURE environment variable platform designation,
make it lowercase, and unify output folder naming.
Signed-off-by: Simon Rozman <simon@rozman.si>
MINIPORT_OID_REQUEST handler should return:
- NDIS_STATUS_NOT_SUPPORTED => NDIS_STATUS_INVALID_OID if the OID
request was not recognized.
- NDIS_STATUS_INVALID_OID => NDIS_STATUS_NOT_SUPPORTED if particular OID
is not supported.
- NDIS_STATUS_INVALID_LENGTH => NDIS_STATUS_BUFFER_TOO_SHORT if output
buffer is too short.
Signed-off-by: Simon Rozman <simon@rozman.si>
