From: Jan Beulich Subject: domain: locking for iomem_caps accesses In order to be able to pull at least the XEN_DOMCTL_iomem_mapping handling out of the domctl-locked region, a separate (per-domain) lock is needed to synchronize in particular with XEN_DOMCTL_iomem_permission. Locking is added only as far as domctl-s are affected. Uses presently outside of the domctl lock may want dealing with subsequently (perhaps limited to non-__init code). This is part of XSA-492. Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -332,10 +332,15 @@ static int late_hwdom_init(struct domain * may be modified after this hypercall returns if a more complex * device model is desired. */ + write_lock(&dom0->caps_lock); rangeset_swap(d->irq_caps, dom0->irq_caps); rangeset_swap(d->iomem_caps, dom0->iomem_caps); #ifdef CONFIG_X86 rangeset_swap(d->arch.ioport_caps, dom0->arch.ioport_caps); +#endif + write_unlock(&dom0->caps_lock); + +#ifdef CONFIG_X86 setup_io_bitmap(d); setup_io_bitmap(dom0); #endif @@ -631,6 +636,7 @@ struct domain *domain_create(domid_t dom spin_lock_init_prof(d, domain_lock); spin_lock_init_prof(d, page_alloc_lock); spin_lock_init(&d->hypercall_deadlock_mutex); + rwlock_init(&d->caps_lock); INIT_PAGE_LIST_HEAD(&d->page_list); INIT_PAGE_LIST_HEAD(&d->extra_page_list); INIT_PAGE_LIST_HEAD(&d->xenpage_list); --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -278,6 +278,35 @@ static struct vnuma_info *vnuma_init(con return ERR_PTR(ret); } +void iocaps_double_lock(struct domain *d, bool write) +{ + struct domain *currd = current->domain; + + if ( d->domain_id > currd->domain_id ) + read_lock(&currd->caps_lock); + + if ( write ) + write_lock(&d->caps_lock); + else + read_lock(&d->caps_lock); + + if ( d->domain_id < currd->domain_id ) + read_lock(&currd->caps_lock); +} + +void iocaps_double_unlock(struct domain *d, bool write) +{ + struct domain *currd = current->domain; + + if ( d != currd ) + read_unlock(&currd->caps_lock); + + if ( write ) + write_unlock(&d->caps_lock); + else + read_unlock(&d->caps_lock); +} + long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) { long ret = 0; @@ -689,6 +718,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */ break; + iocaps_double_lock(d, true); + if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) || xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, allow) ) @@ -697,6 +728,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1); else ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1); + + iocaps_double_unlock(d, true); break; } @@ -721,19 +754,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe break; #endif + iocaps_double_lock(d, false); + ret = -EPERM; if ( !iomem_access_permitted(current->domain, mfn, mfn_end) || - !iomem_access_permitted(d, mfn, mfn_end) ) - break; - - ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add); - if ( ret ) - break; - - if ( !paging_mode_translate(d) ) - break; - - if ( add ) + !iomem_access_permitted(d, mfn, mfn_end) || + (ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add)) || + !paging_mode_translate(d) ) + /* Nothing. */; + else if ( add ) { printk(XENLOG_G_DEBUG "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n", @@ -757,6 +786,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe "memory_map: error %ld removing dom%d access to [%lx,%lx]\n", ret, d->domain_id, mfn, mfn_end); } + + iocaps_double_unlock(d, false); break; } --- a/xen/include/xen/iocap.h +++ b/xen/include/xen/iocap.h @@ -12,6 +12,9 @@ #include #include +void iocaps_double_lock(struct domain *d, bool write); +void iocaps_double_unlock(struct domain *d, bool write); + static inline int iomem_permit_access(struct domain *d, unsigned long s, unsigned long e) { --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -493,6 +493,7 @@ struct domain #endif /* I/O capabilities (access to IRQs and memory-mapped I/O). */ + rwlock_t caps_lock; struct rangeset *iomem_caps; struct rangeset *irq_caps;