Re: [Xen-devel] [PATCH] NPT: temporarily retain page table mapping in do_recalc()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-06 at 11:02 +0100, Jan Beulich wrote:
> >>> On 06.05.14 at 11:54, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 05/05/14 11:31, Jan Beulich wrote:
> >> Commit b3e024f3 ("x86/NPT: don't walk page tables when changing types
> >> on a range") neglected the fact that p2m_next_level() replaces the
> >> previous level's mapping with the new level's one, hence dereferencing
> >> a stale pointer the translation for which may no longer be available
> >> (timing dependent). Add a parameter to that function allowing the
> >> caller to request that the mapping be retained (the unmapping will be
> >> taken care of by the caller then).
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Given the new API for p2m_next_level(), it is important to note that the
> > mapping can only be safely left in place on the success path.  In the
> > case of an error the page must be unmapped even if 'unmap' is false.
> > 
> > While the current behaviour is safe, all it would take is an innocent
> > refactoring of the error paths to invalidate this requirement.
> 
> But that's not a correct description of how this behaves: The
> function _always_ leaves a mapping in place. The new parameter
> is used to tell it to return with _two_ established mappings - the
> one that was there on entry and the one needed for the next
> level. Consequently no error path should ever reach this map/
> unmap pair near the end of the function.
> 

That was my conclusion when reviewing too. It took a bit of time to
convince myself it was true, but it was.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel