Re: [Xen-devel] [V6 PATCH 6.2/7] pvh dom0: Add and remove foreign pages

[Tim Deegan <tim@xxxxxxx>]

At 17:47 -0800 on 10 Dec (1386694075), Mukesh Rathor wrote:
> On Wed, 11 Dec 2013 01:35:08 +0000
> Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> 
> > >       unsigned long mfn = gmfn_to_mfn(d, gfn);
> > >
> > > -    ASSERT(t == NULL);
> > > +    ASSERT(*t == INT_MAX);
> > 
> > There is various place where get_page_from_gfn where t == NULL. With 
> > this solution it will segfault every time.
> > 
> > I would do something like that:
> >    if (*t)
> >      t = INT_MAX;
> 
> here's updated:
> ------------
> 
> In this patch, a new function, p2m_add_foreign(), is added
> to map pages from foreign guest into current dom0 for domU creation.
> Such pages are typed p2m_map_foreign. Another function
> p2m_remove_foreign() is added to remove such pages. Note, in
> the remove path, we must release the refcount that was taken during
> the map phase. This is done in p2m_remove_page, which also addresses
> releasing of refcnt when the domain is destroyed.

Did you test that?  I don't think it can be true.

Maybe I wasn't clear last time: this refcount is effectively held by
the presence of a foreign mapping in a p2m entry.  AFAICT the only
properly safe way to make sure that broken guest/tools behaviour can't
mess up Xen's internal refcounting is to have the ref be taken and
dropped at the time that the entry itelf is written/replaced, e.g.
ept_set_entry() (or maybe atomic_write_ept_entry()) on EPT and
paging_write_p2m_entry() on NPT/shadow.

Trying to find all the higher-level operations that might cause
foreign mappings to be inserted/removed is going to be difficult and
fragile.

You'll also need to handle domain teardown, which right now just frees
all the memory holding the p2m tables (see p2m_teardown()).  That will
need somehow to check those tables for valid foreign mappings and DTRT
about them.

Tim.


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