<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Sep 27, 2016 at 3:24 PM, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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 <<a href="mailto:sstutsma@redhat.com">sstutsma@redhat.com</a>> Signed-off-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com">sameeh@daynix.com</a>> --- 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); </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">sameeh@daynix.com</a>> </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) { + 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 </blockquote></div> <div> </div>-- <div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Respectfully, <div style="font-size:12.8px;color:rgb(136,136,136)">Sameeh Jubran</div><div style="font-size:12.8px;color:rgb(136,136,136)"><a href="https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a" title="View public profile" name="UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_14e2c1de96f8c195_UNIQUE_ID_SafeHtmlFilter_SafeHtmlFilter_SafeHtmlFilter_webProfileURL" style="color:rgb(17,85,204);margin:0px;padding:0px;border-width:0px;outline:none;vertical-align:baseline;text-decoration:none" target="_blank">Linkedin</a> </div><div style="font-size:12.8px;color:rgb(136,136,136)">Junior Software Engineer @ <a href="http://www.daynix.com" target="_blank">Daynix</a>.</div></div></div></div></div></div></div> </div></div>