[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 6/7] x86/mm: handle foreign mappings in p2m_entry_modify

On Thu, Feb 14, 2019 at 04:25:49AM -0700, Jan Beulich wrote:
> >>> On 11.02.19 at 18:46, <roger.pau@xxxxxxxxxx> wrote:
> > @@ -948,6 +951,11 @@ static inline void p2m_entry_modify(struct p2m_domain 
> > *p2m, p2m_type_t nt,
> >          p2m->ioreq.entry_count++;
> >          break;
> >  
> > +    case p2m_map_foreign:
> > +        BUG_ON(!mfn_valid(nfn) ||
> > +               !page_get_owner_and_reference(mfn_to_page(nfn)));
> > +        break;
> Asserting that the passed in MFN is valid is fine. Asserting that a
> reference can be got is not, as this sets us up for a DoS in case
> of a refcount overflow, or the page having got ballooned out by
> its owner. That is, the issue of you folding the two original calls
> into one is wider than just the two distinct error codes getting lost
> that were previously produced - you can't (currently) report up
> any error from this low layer. (And I'm sorry, I should have noticed
> this on v1 already.)

What about using something like:

case p2m_map_foreign:

    if ( !page_get_owner_and_reference(mfn_to_page(nfn)) )


It's not strictly worse than what's currently done in EPT code, but I
agree it should be improved.

Improving this will mean modifying the write_p2m_entry hook, and all
the callers, together with fixing the handling of the return code from
atomic_write_ept_entry, which is currently only asserted.

Thanks, Roger.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.