Re: [Xen-devel] [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Tim Deegan <tim@xxxxxxx>]

At 18:19 +0000 on 20 Jan (1421774373), Andrew Cooper wrote:
> On 20/01/15 17:05, Roger Pau Monne wrote:
> > Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
> > would have access to the full MMIO range.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> > Changes since v1:
> >  - Use the newly introduced p2m_access_t to set the access type.
> >  - Don't add a next label.
> > ---
> >  xen/arch/x86/domain_build.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> > index f687c78..41d2541 100644
> > --- a/xen/arch/x86/domain_build.c
> > +++ b/xen/arch/x86/domain_build.c
> > @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain 
> > *d, unsigned long gfn,
> >                                         unsigned long mfn, unsigned long 
> > nr_mfns)
> >  {
> >      unsigned long i;
> > +    mfn_t omfn;
> > +    p2m_type_t t;
> > +    p2m_access_t a;
> >      int rc;
> >  
> >      for ( i = 0; i < nr_mfns; i++ )
> >      {
> > -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
> > -                                      p2m_get_hostp2m(d)->default_access)) 
> > )
> > +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
> > +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
> > +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), 
> > PAGE_ORDER_4K);
> > +            continue;
> > +        }
> 
> This suggests a design flaw (possibly pre-existing).  We should not be
> removing physmap entries in pvh_add_mem_mapping(), nor should we be a
> position to need to revoke physmap entries during domain build.
> 
> If there is anything needing revoking at this stage, it should not have
> been added earlier.

+1.  ISTR saying something like this before. 

Tim.

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