Owen Smith [Thu, 12 Aug 2021 12:39:07 +0000 (13:39 +0100)]
Fix CodeQL warnings
- ExAllocatePoolWithTag is deprecated in Win10 2004, use
ExAllocatePoolUninitialized instead
- QueryCapabilities structure contains padding bytes, using
__AllocatePoolWithTag zeros this memory
Owen Smith [Thu, 12 Aug 2021 12:39:06 +0000 (13:39 +0100)]
Fix SDV/CodeQL log generation
- sarif files need to be stored with SDV logs when generating the DVL file
- Disable PREFast and CodeAnalysis by default
- Run a seperate CodeAnalysis build after SDV, but before generating DVL file
DVL file should contain multiple summary lines for SDV, at least 1 line
for CodeAnalysis and at least 1 line for Semmle (CodeQL)
Owen Smith [Thu, 12 Aug 2021 12:39:05 +0000 (13:39 +0100)]
Fix build with later WDKs
- Adds alias for GetProjectInfoForReference target to version.vcxproj
Later kits seemed to have renamed the build target, and will fail without
this alias target.
- Adds "/fd sha256" to signtool command line
WDK 20344 and later require binaries signed with a SHA256 file digest, or
the build outputs are deleted
Signed-off-by: Owen Smith <owen.smith@citrix.com>
- Squash warnings 4061 and 4062 as they lead to a lot of bogus noise.
- Add a somewhat pointless ASSERT for a condition that has already been
tested to stop the compiler complaining.
Martin Harvey [Wed, 28 Jul 2021 14:02:04 +0000 (15:02 +0100)]
Prevent NULL MDL's from being queued for transmission.
Under conditions of high load and low resources, it was
possible for NDIS (in combination with overlying drivers) to send
NET_BUFFER_LIST structures containing NULL MDL's for transmission.
This resulted in an immediate bugcheck.
This patch prevents NULL MDL's from being sent to XenVif.
Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
Owen Smith [Fri, 5 Mar 2021 10:15:49 +0000 (10:15 +0000)]
Add CodeQL build stage
CodeQL logs will be required for future WHQL submissions. Add a stage
that generates the required SARIF files. CodeQL is a semantic code
analysis engine, which will highlight vunerabilities that will need
fixing.
In order to use CodeQL, the CodeQL binaries must be on the path and the
Windows-Driver-Developer-Supplemental-Tools must be on the path defined
by the CODEQL_QUERY_SUITE environment variable (if defined), or under
the parent folder (if CODEQL_QUERY_SUITE variable is not defined)
Note: Due to the way the codeql command line is built, using quotes in a
MSBuild command line is not possible, so generate a batch file to wrap
the command line.
Inherit versioning info from environment if present
As the drivers stabilize and mature, there is an ever-growing
chance that other opensource virtualization projects will adopt
them. Allow external projects to inject their own versioning
into the drivers instead of hardcoding the latest winpv version.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com> Acked-by: Owen Smith <owen.smith@citrix.com>
Often times we only need to build a driver for a single
targeted architecture. Continue to build both by default,
but allow the user to specify one if desired.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Use [string]::IsNullOrEmpty($Arch)
These bugchecks have been observed in recent updates of Server 2019.
This patch, rather than replacing calls to MmAllocatePagesForMdlEx() with
calls to MmMapLockedPagesSpecifyCache(), just avoids passing
MM_DONT_ZERO_ALLOCATION to work round the bug.
The patch instead passes MM_ALLOCATE_FULLY_REQUIRED, which arguably should
have always been passed for allocations larger than a single page. It also
fixes a formatting issue.
NOTE: Nothing in XENNET currently calls __AllocatePages() so this patch is
not strictly necessary. However, in case a caller is added in future,
it is best to keep util.h in sync with the other drivers.
Reported-by: Jan Bakuwel <jan.bakuwel@gmail.com> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Owen Smith [Fri, 14 Jun 2019 16:32:33 +0000 (17:32 +0100)]
Add PowerShell build scripts, version.vcxproj
Based on the sequence of commits to xenbus, add powershell scripts to
build the solution using the EWDK
version.vcxproj generates versioned files (version.h and xennet.inf) using
scripts/genfiles.ps1
Strips duplicated functionality from build.py to produce consistant
builds between python and powershell.
Owen Smith [Fri, 24 Aug 2018 16:46:43 +0000 (17:46 +0100)]
Conditionally package DPInst
Since DPInst.exe is not shipped with the Windows Driver Kit 10, an
environment variable must point to local copies. Make the inclusion of
DPInst conditional on DPINST_REDIST being defined and that path
existing. This simplifies building packages which do not require DPInst
for installation, and removes a required step to create a working build.
CL emits a warning about every place that will get spectre mitigation
when compiled with /Qspectre. Even if this option is already used. This
breaks the build, as warnings are treated as errors in xennet.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Also disabled the warning in the co-installer build.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 25 Jan 2018 12:51:06 +0000 (12:51 +0000)]
Update util.h
XENNET does not use much of the functionality in util.h, including the
__AllocatePages()/__FreePages() functions modified by this change, however
it is good to keep the header in-sync with the other drivers.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 8 Feb 2017 10:47:06 +0000 (10:47 +0000)]
Fix low resources packet leak
When the low resources limit is hit then the NDIS_RECEIVE_FLAGS_RESOURCES
is passed to NdisMIndicateReceiveNetBufferLists(), which means that the
calling code can assume the NET_BUFFER_LIST is immediately released.
The code therefore attempts to immediately release the chain of
NET_BUFFER_LIST, but because __IndicateReceiveNetBufferLists() segmented
the chain, only the first one is actually released. This leads to a
resource leak which also prevents XENVIF from shutting down correctly
(waiting for the leaked packets to be returned).
This patch fixes the issue by handling the release of individual
NET_BUFFER_LISTs directly in __IndicateReceiveNetBufferLists(), if the
NDIS_RECEIVE_FLAGS_RESOURCES is set.
Reported-by: Martin Cerveny <martin@c-home.cz> Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Tested-by: Martin Cerveny <martin@c-home.cz>
Paul Durrant [Wed, 14 Dec 2016 15:58:56 +0000 (15:58 +0000)]
Fixes for VS2015/WDK10 build
The package build was not working correctly and caused the overall build
to fail.
At least part of the reason for this is that Microsoft, in their infinite
wisdom, have removed the DIFx redist from WDK10. This patch makes use of
a new environment variable 'DPINST_REDIST' to find the copy of dpinst.exe
to package such that this can be pointed at an older WDK or alternative
location where dpinst.exe can be found.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 13 Dec 2016 10:13:48 +0000 (10:13 +0000)]
Add support for building under VS2015/WDK10
Moving to the new toolchain also threw up a few new warnings, which this
patch either fixes or squashes. Also, SDV appears to be fragile in new
ways (and whinge about some new things) so there are fixes for that too.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Fri, 4 Nov 2016 16:02:07 +0000 (16:02 +0000)]
Avoid ASSERTion failure caused by sample ordering reversal
In __ReceiverPushPackets() the value of Receiver->Indicated is sampled
prior to the value of Receiver->Returned. This allows packets to be
indicated and returned on other CPUs between the two sample points
leading to an ASSERTion failure because the value of Returned will be
greater than that of Indicated.
The solution is simply to reverse the sample ordering.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 3 Nov 2016 16:48:22 +0000 (16:48 +0000)]
Fix NdisTest 6.5 OffloadLSO test on Server 2016
This test will cause assertion failures on the server end on Server 2016
when it attempts to send LSOv2 packets to an adapter where LSOv2 has
been disabled. These packets must be binned otherwise the server end
barfs on the fact they have bad checksums.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 22 Sep 2016 10:01:03 +0000 (11:01 +0100)]
Shim NdisMIndicateReceiveNetBufferLists() to avoid multiple indications
It seems that, on some versions of Windows, something in the network
stack does not work properly with receive indications of more than
one NET_BUFFER_LIST at once.
This patch shims NdisMIndicateReceiveNetBufferLists() to iterate over
a chain of NET_BUFFER_LISTS and indicate them separately. Hopefully
this workaround can be removed in future.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 22 Sep 2016 10:07:17 +0000 (11:07 +0100)]
Make sure that a queue of received packets is always pushed
It's possible that the last received packet may suffer an allocation
failure during processing and, as the code stands, any previously
received packets may then not be indicated to the stack.
This patch makes sure that the '!More' condition always results in an
indication, regardless of whether there is an allocation failure.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 7 Sep 2016 15:48:22 +0000 (16:48 +0100)]
Revert all settings stealing code
This patch accompanies commit bf92f4b7 to XENVIF. That patch reverts
XENVIF to using the settings copy mechanism employed in the 8.1 driver,
so this patch accordingly removes all relevant modifications to the
XENNET co-installer.
The patches reverted are:
9695e3bd "Re-instate code network settings code in the co-installer" 59901522 "Remove code to clear stolen stack binding" ed747f69 "Clear stolen linkage on device removal"
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Mon, 15 Aug 2016 13:07:00 +0000 (14:07 +0100)]
Make use of batching support
XENVIF_VIF_INTERFACE version 7 adds support for batch indications on both
the transmit and receive side. This patch imports the updated interface
header and makes use of this new functionality.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 17 Aug 2016 10:22:29 +0000 (11:22 +0100)]
Remove update of defunct statistic
Nothing looks a the 'InNDISMax' value, yet the receiver code still jumps
through potentially performance damaging hoops to update it. This patch
finally blows it away.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 11 Aug 2016 14:56:47 +0000 (15:56 +0100)]
Re-instate code network settings code in the co-installer
This functionality was moved into XENVIF when it transpired that it did not
work with Windows 10. However, attempting to mess with network settings in
a driver has also proved to have problematic corner cases.
This patch re-instates old code but changes the mechnism for acquiring
network settings from an emulated device. Instead of attempting to copy
settings to a new stack binding (which failed on Windows 10 because the
stack binding was not set up), the stack binding of the emulated device is
cloned. This is done in post-install phase if the emulated device is online
(otherwise Windows will refuse to start the PV device) or in pre-install
phase if the emulated device is offline, as is the case when re-installing
XENNET. This appears to work reliably across all variants of Windows.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 26 Jul 2016 08:22:37 +0000 (09:22 +0100)]
Fix VS2012 builds
Building for OS earlier than Windows 7 now requires use of procgrp.lib
since KeGetCurrentProcessorEx() is used. Since the base OS of VS2012 builds
is Vista, these have been failing since commit 8cb5c156.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 19 May 2016 11:28:04 +0000 (12:28 +0100)]
Avoid lock contention between receiving CPUs
The single PutList/GetList for cached NET_BUFFER_LIST structures is
currently a point of contention. This patch gets rid of the contention
by keeping per-CPU GetLists.
To access the definition of HVM_MAX_VCPUS, the hvm_info_table.h header
needs to be pulled in the from the Xen source base. This patch therefore
adds the infrastructure for pulling in Xen headers.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Fri, 4 Mar 2016 11:10:33 +0000 (11:10 +0000)]
Clear stolen linkage on device removal
Commit 04c391d9 "Replace copying network settings with identity stealing" to
XENVIF changed the way that network settings are preserved when replacing
an emulated NIC with a PV network device. This change means that both the
emulated NIC and PV device have the same binding to the Windows network
stack. Thus, if we do not destroy that binding prior to uninstallation of
the PV network driver, the stack will be torn down by the network class
uninstall code rather than left in place for the emulated NIC to use after
reboot.
This patch hence adds code to the XENNET co-installer to remove stolen
linkage information from the registry in the pre-remove phase.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 30 Mar 2016 11:01:26 +0000 (12:01 +0100)]
BootFlags should be a parameter, not a subkey
A misplaced comma in the INF file means that BootFlags is being created as
a subkey under the service key with a default DWORD value, rather than a
DWORD value with the name 'BootFlags'.
This patch moves the comma and fixes the problem.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 24 Feb 2016 10:35:12 +0000 (10:35 +0000)]
Fix a typo that causes 'low resources' to erroneously kick in
This patch fixes a typo which caused XENNET to operate in a 'low resources'
situation most of the time. This was almost certainly causing performance
issues and, oddly, caused the NDISTest 6.5 OffloadChecksum test to fail...
but only on Windows 8.0.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Mon, 1 Feb 2016 15:45:14 +0000 (15:45 +0000)]
Fix a warning in the NDISTest 6.0 2c_OidsNdisRequest test
The format of the response to the OID_GEN_VENDOR_DRIVER_VERSION request was
incorrect, and also the OID_GEN_DRIVER_VERSION response has been lying since
the change to NDIS 6.1.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Mon, 1 Feb 2016 11:06:03 +0000 (11:06 +0000)]
Fix NDISTest 6.5 OffloadLSO test
There are more structure size issues lurking that cause this logo test to
fail. This patch fixes those, others that were found along the way and also
makes sure that any attempt to set IPsecV2 offload parameters fails (since
it is not supported).
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 28 Jan 2016 17:59:26 +0000 (17:59 +0000)]
Use a locally defined StringPrintf
Use of the _snprintf_s symbol fails on Windows 10, where it appears not to
be exported from ntdll. So, for portability's sake it is more robust to
implement a StringPrintf function locally.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 28 Jan 2016 09:57:24 +0000 (09:57 +0000)]
Stash a pointer to _snprintf_s in the ADAPTER structure
It is problematic if we don't, as on resume from suspend we need to
re-acquire the function pointer and this seems to lead to a BSOD, despite
there being no documentation to suggest that use of the aux_klib functions
at DISPATCH_LEVEL is disallowed.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Wed, 27 Jan 2016 14:52:57 +0000 (14:52 +0000)]
Stop using RtlStringCbPrintfA()
It appears that linking an NDIS driver with any ntstrsafe function (when
building with VS2012 and the 8.0 WDK at least) has the side effect of
requiring use of DbgPrint(). Whilst this may not sound like a problem, it
is. This is because the NDISTest 6.0 1c_KernelCalls test insists that any
use of DbgPrint is illegal and so it's impossible to logo an NDIS driver
using ntstrsafe functions.
This patch works around the issue by dynamically linking (at run-time) to
the ntdll _snprintf_s function as a replacement for calls to
RtlStringCbPrintfA(). This entails importing the dynamic linking code from
XENVIF since MmGetSystemRoutineAddress() doesn't seem to work for that
symbol.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 26 Jan 2016 11:34:47 +0000 (11:34 +0000)]
Fix multiple completion vulnerability in transmit code
My previous patch 7c3365d5 "Make transmitter robust against a possible
completion race" did not fix the problem. There is still the possibility
that a NET_BUFFER_LIST containing multiple NET_BUFFERs could lead to
multiple completions if the underlying transmit completes quickly (or indeed
synchrnously). This is because a reference is taken before sending each
NET_BUFFER but, if that transmission completes immediately the reference is
dropped back to zero (leading to the NET_BUFFER_LIST being completed) before
the reference is taken for the next NET_BUFFER.
This patch therefore takes an extra reference before sending any NET_BUFFERs
and then drops it when there are no more NET_BUFFERs to send. This ensures
that the reference count on the NET_BUFFER_LIST can only fall to zero once
the whole thing has been processed.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 19 Jan 2016 11:21:28 +0000 (11:21 +0000)]
Make transmitter robust against a possible completion race
It's possible that a transmission invoked on one CPU may complete on another
before the transmission invocation as finished. Therefore, once we have queued
a NET_BUFFER for transmission we should not use it's metadata again (including
the 'next' pointer).
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 19 Jan 2016 09:31:45 +0000 (09:31 +0000)]
Remove header/data split support
During logo testing it became apparent that:
a) This is predicated on support VLAN ID, which is not something we really
want in a PV driver (since the backend network may actually be a VLAN).
b) Turning on Viridian flags causes NDIS to stop enabling header/data split.
(There's a call to NdisGetHypervisorInfo() in NdisMSetHDSplitAttributes()).
Issue a) makes it undesirable to implement and b) makes it pretty much
pointless having the code around. Hence this patch removes it.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Mon, 18 Jan 2016 17:44:01 +0000 (17:44 +0000)]
Fix various structure sizes
NDIS defines 'sizeof' values for various attribute/parameter structures
which are documented for use with different versions of NDIS. This patch
makes sure we are using these defined values in appropriate places.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 10 Dec 2015 11:28:17 +0000 (11:28 +0000)]
Re-synchronize util.h with XENBUS and use __toupper()
A recent patch introduced __tolower() and __toupper() into util.h as
replacements for tolower() and toupper() respectively as the latter do
a hidden conversion to Unicode which make then unsafe at any IRQL other
than PASSIVE_LEVEL.
This patch imports util.h from XENBUS and fixes other code to be compatible.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 8 Dec 2015 15:06:51 +0000 (15:06 +0000)]
Don't get XENBUS_SUSPEND callback registration on AdapterCount
Instances will not necessarily come and go in order so we can't know that
the last instance to go will be the one the registered the callback.
Instead register a callback for every instance and just use AdapterCount to
decide which instance writes distribution information to xenstore.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Fri, 4 Dec 2015 12:22:08 +0000 (12:22 +0000)]
Move to XENVIF VIF interface version 4
This means we can remove a lot of complexity and crucially we no longer
need to use the XENBUS_CACHE interface, which means we can avoid the race
introduced by commit 026aa32c "Make sure XENBUS interfaces are released
when going into S4".
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 3 Dec 2015 12:55:31 +0000 (12:55 +0000)]
Make sure XENBUS interfaces are released when going into S4
Because a transition into and out of S4 means a new domain is built, it's
crucial that all XENBUS interfaces are released (so that things like
event channels, grant tables and the xenstore ring get re-constructed).
This patch fixes code paths where this was not being done. It also adds
some more logging during AdapterEnable/Disable and when moving between
D0 and D3.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Thu, 22 Oct 2015 14:50:58 +0000 (15:50 +0100)]
Add a registry override to veto driver installations
There are certain cases where a local installer package may wish to
prevent Windows Update installations of drivers. This can be achieved by
having the co-installer check for a registry value and fail it's pre-install
phase if the value is present.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 28 Jul 2015 16:27:04 +0000 (17:27 +0100)]
Add header/data split support
Now that XENNET is using NDIS 6.1, implement header/data split. See
NDIS documentation for more details. This requires using version 3 of the
VIF interface from XENVIF to support the requirements for data MDL
backfill.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 8 Sep 2015 16:20:57 +0000 (17:20 +0100)]
Parameterize vendor prefix and PCI device id
The XenServer PV vendor prefix ('XS') and PCI device (C000) are still
hard-coded into the XENNET package. These need to be stripped out and
replaced by values that can be customized at build time. This patch does
that.
The patch also reverts to building version.h and customizing xennet.inf
directly in build.py.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
projects / pvdrivers / win / xennet.git / log