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

[Javier Celaya]

El mar, 20-09-2016 a las 15:56 +0300, Dmitry Fleytman escribió:
> > 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/dn65336
> 9(v=vs.85).aspx 
> 
> Dmitry

+1




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



_______________________________________________
Spice-devel mailing list
Spice-devel at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel