<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 27, 2016 at 3:24 PM, Frediano Ziglio <span dir="ltr"><<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">Disable execution bit on mapping improving security.<br>
<br>
MmMapIoSpaceEx is available only in Windows 10 thus<br>
the macros are used.<br>
<br>
</span><span class="gmail-">Based on a patch by Sandy Stutsman <<a href="mailto:sstutsma@redhat.com" target="_blank">sstutsma@redhat.com</a>><br>
<br>
Signed-off-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com" target="_blank">sameeh@daynix.com</a>><br>
---<br>
</span> qxldod/QxlDod.cpp | 16 +++++++------<br>
qxldod/compat.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++<br>
qxldod/compat.h | 10 +++++++++<br>
qxldod/qxldod.vcxproj | 2 ++<br>
qxldod/qxldod.vcxproj.filters | 6 +++++<br>
5 files changed, 79 insertions(+), 7 deletions(-)<br>
create mode 100755 qxldod/compat.cpp<br>
create mode 100755 qxldod/compat.h<br><br>
I send this as a RFC.<br><br>
Changes from v5:<br>
- detect function dynamically and call the proper one.<br><br>
This avoid to have to maintain 2 binaries for Windows 8 and Windows 10.<br>
Perhaps the way dynamic detect is done is a bit overkilling as<br>
setting up when driver is initialized and a simple if (pMmMapIoSpaceEx)<br>
could make the function easier.<br>
I like to have compatibility code in separate files.<br>
License headers for new files are missing.<br>
This code should go in the paged area.<br><br>
diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp<br>
index 0d91f92..3542ab4 100755<br>
--- a/qxldod/QxlDod.cpp<br>
+++ b/qxldod/QxlDod.cpp<br>
@@ -1,6 +1,7 @@<br>
#include "driver.h"<br>
#include "qxldod.h"<br>
#include "qxl_windows.h"<br>
+#include "compat.h"<br><br>
#pragma code_seg(push)<br>
#pragma code_seg()<br>
@@ -1998,17 +1999,18 @@ MapFrameBuffer(<br>
return STATUS_INVALID_PARAMETER;<br>
}<br><br>
- *VirtualAddress = MmMapIoSpace(PhysicalAddress,<br>
- Length,<br>
- MmWriteCombined);<br>
+ *VirtualAddress = MapIoSpace(PhysicalAddress,<br>
+ Length,<br>
+ MmWriteCombined,<br>
+ PAGE_WRITECOMBINE | PAGE_READWRITE);<br><span class="gmail-"> if (*VirtualAddress == NULL)<br>
{<br>
// The underlying call to MmMapIoSpace failed. This may be because, MmWriteCombined<br>
// isn't supported, so try again with MmNonCached<br>
-<br>
</span>- *VirtualAddress = MmMapIoSpace(PhysicalAddress,<br>
- Length,<br>
- MmNonCached);<br>
+ *VirtualAddress = MapIoSpace(PhysicalAddress,<br>
+ Length,<br>
+ MmNonCached,<br>
+ PAGE_NOCACHE | PAGE_READWRITE);<br><span class="gmail-"> if (*VirtualAddress == NULL)<br>
{<br>
DbgPrint(TRACE_LEVEL_ERROR, ("MmMapIoSpace returned a NULL buffer when trying to allocate %lu bytes", Length));<br>
</span>diff --git a/qxldod/compat.cpp b/qxldod/compat.cpp<br>
new file mode 100755<br>
index 0000000..7ca7ae5<br>
--- /dev/null<br>
+++ b/qxldod/compat.cpp<br>
@@ -0,0 +1,52 @@<br>
+#include "driver.h"<br>
+#include "compat.h"<br>
+<br>
+static MapIoSpaceFunc DetectMapIoSpace;<br>
+MapIoSpaceFunc *MapIoSpace = DetectMapIoSpace;<br>
+<br>
+typedef NTKERNELAPI PVOID MmMapIoSpaceExFunc(<br>
+ _In_ PHYSICAL_ADDRESS PhysicalAddress,<br>
+ _In_ SIZE_T NumberOfBytes,<br>
+ _In_ ULONG Protect<br>
+);<br>
+static MmMapIoSpaceExFunc *pMmMapIoSpaceEx;<br>
+<br>
+static PVOID OldMapIoSpace(<br>
+ _In_ PHYSICAL_ADDRESS PhysicalAddress,<br>
+ _In_ SIZE_T NumberOfBytes,<br>
+ _In_ MEMORY_CACHING_TYPE CacheType,<br>
+ _In_ ULONG Protect<br>
+)<br>
+{<br>
+ if (NumberOfBytes == 0) return NULL;</blockquote></div></div></div></blockquote><div><br></div><div>This was only for debuggin, I'll remove it.<br></div><div><br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
+ return MmMapIoSpace(PhysicalAddress, NumberOfBytes, CacheType);<br>
+}<br>
+<br>
+static PVOID NewMapIoSpace(<br>
+ _In_ PHYSICAL_ADDRESS PhysicalAddress,<br>
+ _In_ SIZE_T NumberOfBytes,<br>
+ _In_ MEMORY_CACHING_TYPE CacheType,<br>
+ _In_ ULONG Protect<br>
+)<br>
+{<br>
+ return pMmMapIoSpaceEx(PhysicalAddress, NumberOfBytes, Protect);<br>
+}<br>
+<br>
+static PVOID DetectMapIoSpace(<br>
+ _In_ PHYSICAL_ADDRESS PhysicalAddress,<br>
+ _In_ SIZE_T NumberOfBytes,<br>
+ _In_ MEMORY_CACHING_TYPE CacheType,<br>
+ _In_ ULONG Protect<br>
+)<br>
+{<br>
+ UNICODE_STRING name;<br>
+ RtlInitUnicodeString(&name, L"MmMapIoSpaceEx");<br>
+<br>
+ pMmMapIoSpaceEx = (MmMapIoSpaceExFunc*)MmGetSystemRoutineAddress(&name);<br></blockquote><div>I think it is better to save this pointer instead of calling MmGetSystemRoutineAddress each time</div><div>we call this function.</div><div>Other than that:</div><div>Acked-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com" target="_blank">sameeh@daynix.com</a>></div></div></div></div></blockquote><div><br></div><div>Yes, but this function is called just once.<br></div><div>The function is called only using MapIoSpace pointer which initially is DetectMapIoSpace.<br></div><div>However DetectMapIoSpace change MapIoSpace pointer to be NewMapIoSpace or OldMapIoSpace so<br></div><div>next time you'll call one of these functions.<br></div><div>Somebody could argue that if two thread call MapIoSpace you could enter the function twice but<br></div><div>this is not an issue as the pointer you are going to write to MapIoSpace is the same.<br></div><div>I use this way as usually I replace a new function with a function that detect and implement it if not<br></div><div>available. In this way once is detected is faster as calling the API directly. This as the API import store<br></div><div>a pointer into a variable so you always have the indirect call. This actually could even be faster as some<br></div><div>import way could produce a direct call to an indirect jump.<br></div><div>Yes, pure paranoia!<br></div><div><br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ if (pMmMapIoSpaceEx) {<br>
+ MapIoSpace = NewMapIoSpace;<br>
+ } else {<br>
+ MapIoSpace = OldMapIoSpace;<br>
+ }<br>
+ return MapIoSpace(PhysicalAddress, NumberOfBytes, CacheType, Protect);<br>
+}<br>
diff --git a/qxldod/compat.h b/qxldod/compat.h<br>
new file mode 100755<br>
index 0000000..3f20b81<br>
--- /dev/null<br>
+++ b/qxldod/compat.h<br>
@@ -0,0 +1,10 @@<br>
+#pragma once<br>
+#include "BaseObject.h"<br>
+<br>
+typedef PVOID MapIoSpaceFunc(<br>
+ _In_ PHYSICAL_ADDRESS PhysicalAddress,<br>
+ _In_ SIZE_T NumberOfBytes,<br>
+ _In_ MEMORY_CACHING_TYPE CacheType,<br>
+ _In_ ULONG Protect<br>
+);<br>
+extern MapIoSpaceFunc *MapIoSpace;<br>
diff --git a/qxldod/qxldod.vcxproj b/qxldod/qxldod.vcxproj<br>
index 37d2b38..2c10158 100755<br>
--- a/qxldod/qxldod.vcxproj<br>
+++ b/qxldod/qxldod.vcxproj<br>
@@ -272,12 +272,14 @@<br>
</ItemGroup><br>
<ItemGroup><br>
<ClInclude Include="BaseObject.h" /><br>
+ <ClInclude Include="compat.h" /><br>
<ClInclude Include="driver.h" /><br>
<ClInclude Include="QxlDod.h" /><br>
<ClInclude Include="resource.h" /><br>
</ItemGroup><br>
<ItemGroup><br>
<ClCompile Include="BaseObject.cpp" /><br>
+ <ClCompile Include="compat.cpp" /><br>
<ClCompile Include="driver.cpp" /><br>
<ClCompile Include="mspace.c" /><br>
<ClCompile Include="QxlDod.cpp" /><br>
diff --git a/qxldod/qxldod.vcxproj.filters b/qxldod/qxldod.vcxproj.filters<br>
index bb9daa9..1e86aa6 100755<br>
--- a/qxldod/qxldod.vcxproj.filters<br>
+++ b/qxldod/qxldod.vcxproj.filters<br>
@@ -25,6 +25,9 @@<br>
<ClInclude Include="resource.h"><br>
<Filter>Header Files</Filter><br>
</ClInclude><br>
+ <ClInclude Include="compat.h"><br>
+ <Filter>Header Files</Filter><br>
+ </ClInclude><br>
<ClInclude Include="driver.h"><br>
<Filter>Header Files</Filter><br>
</ClInclude><br>
@@ -36,6 +39,9 @@<br>
<ClCompile Include="BaseObject.cpp"><br>
<Filter>Source Files</Filter><br>
</ClCompile><br>
+ <ClCompile Include="compat.cpp"><br>
+ <Filter>Source Files</Filter><br>
+ </ClCompile><br>
<ClCompile Include="driver.cpp"><br>
<Filter>Source Files</Filter><br>
</ClCompile></blockquote></div></div></div></blockquote><div>I'll send an update adding PAGED_CODE() "calls".<br></div><div><br></div><div>Frediano<br></div><div><br></div></div></body></html>