[Spice-devel] [PATCH qxl-wddm-dod v5.1 1/7] Use MmMapIoSpaceEx instead of MmMapIoSpace

[Frediano Ziglio]

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 <sstutsma at redhat.com>

Signed-off-by: Sameeh Jubran <sameeh at 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);
+    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