[Spice-devel] [PATCH qxl-wddm-dod v6] 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             | 62 +++++++++++++++++++++++++++++++++++++++++++
 qxldod/compat.h               | 10 +++++++
 qxldod/qxldod.vcxproj         |  2 ++
 qxldod/qxldod.vcxproj.filters |  6 +++++
 5 files changed, 89 insertions(+), 7 deletions(-)
 create mode 100644 qxldod/compat.cpp
 create mode 100644 qxldod/compat.h

Changes since v5.1:
- put functions in proper PAGED area;
- disable a warning calling MmMapIoSpace as expected;
- removed debug line.

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 5cfff78..b0bf55e 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()
@@ -1994,17 +1995,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 100644
index 0000000..88f9619
--- /dev/null
+++ b/qxldod/compat.cpp
@@ -0,0 +1,62 @@
+#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;
+
+// all functions in this module are paged
+#pragma code_seg("PAGE")
+
+// we call MmMapIoSpace only if MmMapIoSpaceEx is not present
+// so disable the warning
+#pragma warning(push)
+#pragma warning(disable:30029)
+static PVOID OldMapIoSpace(
+    _In_ PHYSICAL_ADDRESS PhysicalAddress,
+    _In_ SIZE_T           NumberOfBytes,
+    _In_ MEMORY_CACHING_TYPE CacheType,
+    _In_ ULONG            Protect
+)
+{
+    PAGED_CODE();
+    return MmMapIoSpace(PhysicalAddress, NumberOfBytes, CacheType);
+}
+#pragma warning(pop)
+
+static PVOID NewMapIoSpace(
+    _In_ PHYSICAL_ADDRESS PhysicalAddress,
+    _In_ SIZE_T           NumberOfBytes,
+    _In_ MEMORY_CACHING_TYPE CacheType,
+    _In_ ULONG            Protect
+)
+{
+    PAGED_CODE();
+    return pMmMapIoSpaceEx(PhysicalAddress, NumberOfBytes, Protect);
+}
+
+static PVOID DetectMapIoSpace(
+    _In_ PHYSICAL_ADDRESS PhysicalAddress,
+    _In_ SIZE_T           NumberOfBytes,
+    _In_ MEMORY_CACHING_TYPE CacheType,
+    _In_ ULONG            Protect
+)
+{
+    PAGED_CODE();
+    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 100644
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