On Tue, Sep 27, 2016 at 3:24 PM, Frediano Ziglio <fzig...@redhat.com> wrote:
> Disable execution bit on mapping improving security.
>
> MmMapIoSpaceEx is available only in Windows 10 thus
> the macros are used.
>
> Based on a patch by Sandy Stutsman <sstut...@redhat.com>
>
> Signed-off-by: Sameeh Jubran <sam...@daynix.com>
> ---
> qxldod/QxlDod.cpp | 16 +++++++------
> qxldod/compat.cpp | 52 ++++++++++++++++++++++++++++++
> +++++++++++++
> qxldod/compat.h | 10 +++++++++
> qxldod/qxldod.vcxproj | 2 ++
> qxldod/qxldod.vcxproj.filters | 6 +++++
> 5 files changed, 79 insertions(+), 7 deletions(-)
> create mode 100755 qxldod/compat.cpp
> create mode 100755 qxldod/compat.h
>
> I send this as a RFC.
>
> Changes from v5:
> - detect function dynamically and call the proper one.
>
> This avoid to have to maintain 2 binaries for Windows 8 and Windows 10.
> Perhaps the way dynamic detect is done is a bit overkilling as
> setting up when driver is initialized and a simple if (pMmMapIoSpaceEx)
> could make the function easier.
> I like to have compatibility code in separate files.
> License headers for new files are missing.
> This code should go in the paged area.
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 0d91f92..3542ab4 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -1,6 +1,7 @@
> #include "driver.h"
> #include "qxldod.h"
> #include "qxl_windows.h"
> +#include "compat.h"
>
> #pragma code_seg(push)
> #pragma code_seg()
> @@ -1998,17 +1999,18 @@ MapFrameBuffer(
> return STATUS_INVALID_PARAMETER;
> }
>
> - *VirtualAddress = MmMapIoSpace(PhysicalAddress,
> - Length,
> - MmWriteCombined);
> + *VirtualAddress = MapIoSpace(PhysicalAddress,
> + Length,
> + MmWriteCombined,
> + PAGE_WRITECOMBINE | PAGE_READWRITE);
> if (*VirtualAddress == NULL)
> {
> // The underlying call to MmMapIoSpace failed. This may be
> because, MmWriteCombined
> // isn't supported, so try again with MmNonCached
> -
> - *VirtualAddress = MmMapIoSpace(PhysicalAddress,
> - Length,
> - MmNonCached);
> + *VirtualAddress = MapIoSpace(PhysicalAddress,
> + Length,
> + MmNonCached,
> + PAGE_NOCACHE | PAGE_READWRITE);
> if (*VirtualAddress == NULL)
> {
> DbgPrint(TRACE_LEVEL_ERROR, ("MmMapIoSpace returned a NULL
> buffer when trying to allocate %lu bytes", Length));
> diff --git a/qxldod/compat.cpp b/qxldod/compat.cpp
> new file mode 100755
> index 0000000..7ca7ae5
> --- /dev/null
> +++ b/qxldod/compat.cpp
> @@ -0,0 +1,52 @@
> +#include "driver.h"
> +#include "compat.h"
> +
> +static MapIoSpaceFunc DetectMapIoSpace;
> +MapIoSpaceFunc *MapIoSpace = DetectMapIoSpace;
> +
> +typedef NTKERNELAPI PVOID MmMapIoSpaceExFunc(
> + _In_ PHYSICAL_ADDRESS PhysicalAddress,
> + _In_ SIZE_T NumberOfBytes,
> + _In_ ULONG Protect
> +);
> +static MmMapIoSpaceExFunc *pMmMapIoSpaceEx;
> +
> +static PVOID OldMapIoSpace(
> + _In_ PHYSICAL_ADDRESS PhysicalAddress,
> + _In_ SIZE_T NumberOfBytes,
> + _In_ MEMORY_CACHING_TYPE CacheType,
> + _In_ ULONG Protect
> +)
> +{
> + if (NumberOfBytes == 0) return NULL;
> + return MmMapIoSpace(PhysicalAddress, NumberOfBytes, CacheType);
> +}
> +
> +static PVOID NewMapIoSpace(
> + _In_ PHYSICAL_ADDRESS PhysicalAddress,
> + _In_ SIZE_T NumberOfBytes,
> + _In_ MEMORY_CACHING_TYPE CacheType,
> + _In_ ULONG Protect
> +)
> +{
> + return pMmMapIoSpaceEx(PhysicalAddress, NumberOfBytes, Protect);
> +}
> +
> +static PVOID DetectMapIoSpace(
> + _In_ PHYSICAL_ADDRESS PhysicalAddress,
> + _In_ SIZE_T NumberOfBytes,
> + _In_ MEMORY_CACHING_TYPE CacheType,
> + _In_ ULONG Protect
> +)
> +{
> + UNICODE_STRING name;
> + RtlInitUnicodeString(&name, L"MmMapIoSpaceEx");
> +
> + pMmMapIoSpaceEx = (MmMapIoSpaceExFunc*)MmGetSystemRoutineAddress(&
> name);
>
I think it is better to save this pointer instead of calling
MmGetSystemRoutineAddress each time
we call this function.
Other than that:
Acked-by: Sameeh Jubran <sam...@daynix.com>
> + if (pMmMapIoSpaceEx) {
> + MapIoSpace = NewMapIoSpace;
> + } else {
> + MapIoSpace = OldMapIoSpace;
> + }
> + return MapIoSpace(PhysicalAddress, NumberOfBytes, CacheType, Protect);
> +}
> diff --git a/qxldod/compat.h b/qxldod/compat.h
> new file mode 100755
> index 0000000..3f20b81
> --- /dev/null
> +++ b/qxldod/compat.h
> @@ -0,0 +1,10 @@
> +#pragma once
> +#include "BaseObject.h"
> +
> +typedef PVOID MapIoSpaceFunc(
> + _In_ PHYSICAL_ADDRESS PhysicalAddress,
> + _In_ SIZE_T NumberOfBytes,
> + _In_ MEMORY_CACHING_TYPE CacheType,
> + _In_ ULONG Protect
> +);
> +extern MapIoSpaceFunc *MapIoSpace;
> diff --git a/qxldod/qxldod.vcxproj b/qxldod/qxldod.vcxproj
> index 37d2b38..2c10158 100755
> --- a/qxldod/qxldod.vcxproj
> +++ b/qxldod/qxldod.vcxproj
> @@ -272,12 +272,14 @@
> </ItemGroup>
> <ItemGroup>
> <ClInclude Include="BaseObject.h" />
> + <ClInclude Include="compat.h" />
> <ClInclude Include="driver.h" />
> <ClInclude Include="QxlDod.h" />
> <ClInclude Include="resource.h" />
> </ItemGroup>
> <ItemGroup>
> <ClCompile Include="BaseObject.cpp" />
> + <ClCompile Include="compat.cpp" />
> <ClCompile Include="driver.cpp" />
> <ClCompile Include="mspace.c" />
> <ClCompile Include="QxlDod.cpp" />
> diff --git a/qxldod/qxldod.vcxproj.filters b/qxldod/qxldod.vcxproj.filters
> index bb9daa9..1e86aa6 100755
> --- a/qxldod/qxldod.vcxproj.filters
> +++ b/qxldod/qxldod.vcxproj.filters
> @@ -25,6 +25,9 @@
> <ClInclude Include="resource.h">
> <Filter>Header Files</Filter>
> </ClInclude>
> + <ClInclude Include="compat.h">
> + <Filter>Header Files</Filter>
> + </ClInclude>
> <ClInclude Include="driver.h">
> <Filter>Header Files</Filter>
> </ClInclude>
> @@ -36,6 +39,9 @@
> <ClCompile Include="BaseObject.cpp">
> <Filter>Source Files</Filter>
> </ClCompile>
> + <ClCompile Include="compat.cpp">
> + <Filter>Source Files</Filter>
> + </ClCompile>
> <ClCompile Include="driver.cpp">
> <Filter>Source Files</Filter>
> </ClCompile>
> --
> 2.7.4
>
>
--
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Junior Software Engineer @ Daynix <http://www.daynix.com>.*
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
