[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 for-4.6 1/2] xen/pvh: check permissions when adding MMIO regions
On 18/12/14 18:27, 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> > --- > xen/arch/x86/domain_build.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c > index 7a6afea..aa3bf0f 100644 > --- a/xen/arch/x86/domain_build.c > +++ b/xen/arch/x86/domain_build.c > @@ -312,16 +312,47 @@ static __init void pvh_add_mem_mapping(struct domain > *d, unsigned long gfn, > unsigned long mfn, unsigned long > nr_mfns) > { > unsigned long i; > + xenmem_access_t def_access; > + bool_t r_only = false; > int rc; > > for ( i = 0; i < nr_mfns; i++ ) > { > + if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) > + goto next; > + > + if ( rangeset_contains_singleton(mmio_ro_ranges, mfn + i) && !r_only > ) > + { > + rc = p2m_get_mem_access(d, ~0ul, &def_access); > + BUG_ON(rc); > + /* Set default access to read-only */ > + rc = p2m_set_mem_access(d, ~0ul, 0, 0, 0, XENMEM_access_r); > + BUG_ON(rc); > + r_only = true; > + } > + else if ( r_only ) > + { > + /* Set the default back */ > + rc = p2m_set_mem_access(d, ~0ul, 0, 0, 0, def_access); > + BUG_ON(rc); > + r_only = false; > + } Am I missing something obvious, or would all this r_only juggling be far more easy if set_mmio_p2m_entry() had a ro/rw boolean parameter? As these entries are done one at a time, it would seem to be far less error prone to explicitly choose a read-only or read-write mmio mapping, rather than playing with the entire default. > + > if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i))) ) > panic("pvh_add_mem_mapping: gfn:%lx mfn:%lx i:%ld rc:%d\n", > gfn, mfn, i, rc); > + > + next: > if ( !(i & 0xfffff) ) > process_pending_softirqs(); You could remove the next label by moving this clause to the top and adding an i != 0 check. > } > + > + if ( r_only ) > + { > + /* Set the default back */ > + rc = p2m_set_mem_access(d, ~0ul, 0, 0, 0, def_access); This will clobber the p2m default access type based on whether the final mfn is read-only or read-write. ~Andrew > + BUG_ON(rc); > + } > } > > /* _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |