[Spice-devel] [PATCH qxl-wddm-dod v4 03/10] Use MmMapIoSpaceEx instead of MmMapIoSpace

[Dmitry Fleytman]

> On 20 Sep 2016, at 15:46 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
> 
>> On Tue, Sep 20, 2016 at 01:14:10PM +0300, Sameeh Jubran wrote:
>>> On Mon, Sep 12, 2016 at 11:32 AM, Frediano Ziglio <fziglio at redhat.com>
>>> wrote:
>>>> It looks like MmMapIoSpaceEx was introduced with Windows 10 while WDDM
>>>> was
>>>> introduce in Windows 7. Are we going to support only from Windows 10?
>>>> Windows 7 supports WDDM but supports also XPDM so it's supported but
>>>> users of Windows 8 won't have a QXL driver. Are we never going to support
>>>> Windows 8?
>>>> 
>>> 
>>>> 
>>>> I think the reason for this change is that the new function can specify
>>>> additional security, specifically removing the executable bit from page
>>>> mappings. Am I right? Perhaps this should be mentioned in the rationale.
>>> 
>>> I think we can drop this patch if we plan to support Windows 8
>> 
>> It would be easier to make that decision if there was a rationale for
>> the change in the commit log, or if you could answer Frediano's question
>> as to _why_ this commit is changing from one to the other function.
>> 
>> Christophe
>> 
> 
> Looks like WDDM is supported by Windows 8 but not WDDM DOD and we are not
> planning to support full WDDM so would make sense to move to MmMapIoSpaceEx
> is this is increasing security as no execution bit is set on these pages.
> It's not hard to switch it back.

Hi Frediano, 

According to [1] WDDM DOD is supported by Windows 8 and It makes
sense to do at least minimal effort to make this driver work on pre-Win10 systems.

We don’t know what was Sandy’s motivation to migrate to this new function, but
it is preferable indeed w.r.t security concerns and we would like to continue using
it on Windows 10.

We will amend this patch with conditional compilation to make sure
that the best function available is used on on each OS version.

[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn653369(v=vs.85).aspx 

Dmitry

> 
> I would change the commit message as
> 
> -----
> Use MmMapIoSpaceEx instead of MmMapIoSpace
> 
> Disable execution bit on mapping improving security.
> 
> Based on a patch by Sandy Stutsman <sstutsma at redhat.com>
> 
> Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> -----
> 
> and I would remove the ULONG cast.
> If you agree I can push with these changes.
> 
> Frediano