From: Jan Beulich Subject: domctl: handle XEN_DOMCTL_memory_mapping without acquiring domctl lock With dedicated locking added, the domctl lock isn't required here anymore. Move the re-purposed dedicated XSM check as early as possible. Minimal "modernization": Switch "add" to bool and use %pd in log messages. This is part of XSA-492. Fixes: fda49f9b3fbb ("Add build option to allow more hypercalls from stubdoms") Reported-by: Andrew Cooper Signed-off-by: Jan Beulich Acked-by: Daniel P. Smith Reviewed-by: Roger Pau Monné --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -366,6 +366,66 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe goto domctl_out_unlock_domonly; + case XEN_DOMCTL_memory_mapping: + { + unsigned long gfn = op->u.memory_mapping.first_gfn; + unsigned long mfn = op->u.memory_mapping.first_mfn; + unsigned long nr_mfns = op->u.memory_mapping.nr_mfns; + unsigned long mfn_end = mfn + nr_mfns - 1; + bool add = op->u.memory_mapping.add_mapping; + + ret = -EINVAL; + if ( mfn_end < mfn || /* Wrap? */ + ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) || + (gfn + nr_mfns - 1) < gfn ) /* Wrap? */ + goto domctl_out_unlock_domonly; + + ret = xsm_iomem_mapping(XSM_DM_PRIV, d, mfn, mfn_end, add); + if ( ret || !paging_mode_translate(d) ) + goto domctl_out_unlock_domonly; + +#ifndef CONFIG_X86 /* XXX ARM!? */ + ret = -E2BIG; + /* Must break hypercall up as this could take a while. */ + if ( nr_mfns > 64 ) + goto domctl_out_unlock_domonly; +#endif + + iocaps_double_lock(d, false); + + ret = -EPERM; + if ( !iomem_access_permitted(current->domain, mfn, mfn_end) || + !iomem_access_permitted(d, mfn, mfn_end) ) + /* Nothing. */; + else if ( add ) + { + printk(XENLOG_G_DEBUG + "memory_map:add: %pd gfn=%lx mfn=%lx nr=%lx\n", + d, gfn, mfn, nr_mfns); + + ret = map_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn)); + if ( ret < 0 ) + printk(XENLOG_G_WARNING + "memory_map:fail: %pd gfn=%lx mfn=%lx nr=%lx ret:%ld\n", + d, gfn, mfn, nr_mfns, ret); + } + else + { + printk(XENLOG_G_DEBUG + "memory_map:remove: %pd gfn=%lx mfn=%lx nr=%lx\n", + d, gfn, mfn, nr_mfns); + + ret = unmap_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn)); + if ( ret < 0 && is_hardware_domain(current->domain) ) + printk(XENLOG_ERR + "memory_map: error %ld removing %pd access to [%lx,%lx]\n", + ret, d, mfn, mfn_end); + } + + iocaps_double_unlock(d, false); + goto domctl_out_unlock_domonly; + } + default: /* Everything else handled further down. */ break; @@ -744,64 +804,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe break; } - case XEN_DOMCTL_memory_mapping: - { - unsigned long gfn = op->u.memory_mapping.first_gfn; - unsigned long mfn = op->u.memory_mapping.first_mfn; - unsigned long nr_mfns = op->u.memory_mapping.nr_mfns; - unsigned long mfn_end = mfn + nr_mfns - 1; - int add = op->u.memory_mapping.add_mapping; - - ret = -EINVAL; - if ( mfn_end < mfn || /* wrap? */ - ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) || - (gfn + nr_mfns - 1) < gfn ) /* wrap? */ - break; - -#ifndef CONFIG_X86 /* XXX ARM!? */ - ret = -E2BIG; - /* Must break hypercall up as this could take a while. */ - if ( nr_mfns > 64 ) - 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) || - (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", - d->domain_id, gfn, mfn, nr_mfns); - - ret = map_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn)); - if ( ret < 0 ) - printk(XENLOG_G_WARNING - "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n", - d->domain_id, gfn, mfn, nr_mfns, ret); - } - else - { - printk(XENLOG_G_DEBUG - "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n", - d->domain_id, gfn, mfn, nr_mfns); - - ret = unmap_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn)); - if ( ret < 0 && is_hardware_domain(current->domain) ) - printk(XENLOG_ERR - "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; - } - case XEN_DOMCTL_settimeoffset: domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds); break; --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -168,12 +168,12 @@ static XSM_INLINE int cf_check xsm_domct switch ( cmd ) { case XEN_DOMCTL_ioport_mapping: - case XEN_DOMCTL_memory_mapping: case XEN_DOMCTL_bind_pt_irq: case XEN_DOMCTL_unbind_pt_irq: return xsm_default_action(XSM_DM_PRIV, current->domain, d); case XEN_DOMCTL_getdomaininfo: + case XEN_DOMCTL_memory_mapping: ASSERT_UNREACHABLE(); return -EILSEQ; @@ -575,7 +575,7 @@ static XSM_INLINE int cf_check xsm_iomem static XSM_INLINE int cf_check xsm_iomem_mapping( XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow) { - XSM_ASSERT_ACTION(XSM_HOOK); + XSM_ASSERT_ACTION(XSM_DM_PRIV); return xsm_default_action(action, current->domain, d); } --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -680,6 +680,7 @@ static int cf_check flask_domctl(struct /* These have individual XSM hooks and don't make it here. */ case XEN_DOMCTL_getdomaininfo: + case XEN_DOMCTL_memory_mapping: ASSERT_UNREACHABLE(); return -EILSEQ; @@ -687,7 +688,6 @@ static int cf_check flask_domctl(struct case XEN_DOMCTL_scheduler_op: case XEN_DOMCTL_irq_permission: case XEN_DOMCTL_iomem_permission: - case XEN_DOMCTL_memory_mapping: case XEN_DOMCTL_set_target: case XEN_DOMCTL_vm_event_op: