From 9f54e10bf3ab02c8d5dc3253d85c9b3258d1fc88 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Thu, 4 Jun 2026 20:20:44 +0100 Subject: [PATCH] domctl: handle XEN_DOMCTL_io{mem,port}_permission without acquiring domctl lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With dedicated locking added, the domctl lock isn't required here anymore. As the I/O port handling is in arch-specific code (x86 only), no code is being moved, but the 2nd invocation of arch_do_domctl() is re-used. Move the re-purposed dedicated XSM checks as early as possible. This is part of XSA-492. Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné Acked-by: Daniel P. Smith --- xen/arch/x86/domctl.c | 13 +++++++--- xen/common/domctl.c | 54 ++++++++++++++++++++++------------------- xen/include/xsm/dummy.h | 6 +++-- xen/xsm/flask/hooks.c | 4 +-- 4 files changed, 44 insertions(+), 33 deletions(-) diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 66c6bdf0a3..18c19c4b63 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -237,12 +237,17 @@ long arch_do_domctl( unsigned int np = domctl->u.ioport_permission.nr_ports; int allow = domctl->u.ioport_permission.allow_access; + ret = -EINVAL; + if ( (fp + np) <= fp || (fp + np) > MAX_IOPORTS ) + break; + + ret = xsm_ioport_permission(XSM_PRIV, d, fp, fp + np - 1, allow); + if ( ret ) + break; + iocaps_double_lock(d, true); - if ( (fp + np) <= fp || (fp + np) > MAX_IOPORTS ) - ret = -EINVAL; - else if ( !ioports_access_permitted(currd, fp, fp + np - 1) || - xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, allow) ) + if ( !ioports_access_permitted(currd, fp, fp + np - 1) ) ret = -EPERM; else if ( allow ) ret = ioports_permit_access(d, fp, fp + np - 1); diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 10460c1c57..c6c8e0440b 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -376,6 +376,34 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) copyback = true; goto domctl_out_unlock_domonly; + case XEN_DOMCTL_iomem_permission: + { + unsigned long mfn = op->u.iomem_permission.first_mfn; + unsigned long nr_mfns = op->u.iomem_permission.nr_mfns; + bool allow = op->u.iomem_permission.allow_access; + + ret = -EINVAL; + if ( (mfn + nr_mfns - 1) < mfn ) /* Wrap? */ + goto domctl_out_unlock_domonly; + + ret = xsm_iomem_permission(XSM_PRIV, d, mfn, mfn + nr_mfns - 1, allow); + if ( ret ) + goto domctl_out_unlock_domonly; + + iocaps_double_lock(d, true); + + if ( !iomem_access_permitted(current->domain, + mfn, mfn + nr_mfns - 1) ) + ret = -EPERM; + else if ( allow ) + 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); + goto domctl_out_unlock_domonly; + } + case XEN_DOMCTL_memory_mapping: { unsigned long gfn = op->u.memory_mapping.first_gfn; @@ -436,6 +464,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) goto domctl_out_unlock_domonly; } + case XEN_DOMCTL_ioport_permission: case XEN_DOMCTL_ioport_mapping: case XEN_DOMCTL_bind_pt_irq: case XEN_DOMCTL_unbind_pt_irq: @@ -782,31 +811,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) } #endif - case XEN_DOMCTL_iomem_permission: - { - unsigned long mfn = op->u.iomem_permission.first_mfn; - unsigned long nr_mfns = op->u.iomem_permission.nr_mfns; - int allow = op->u.iomem_permission.allow_access; - - ret = -EINVAL; - 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) ) - ret = -EPERM; - else if ( allow ) - 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; - } - case XEN_DOMCTL_settimeoffset: domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds); break; diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 2ab2f5fc52..d3d2a29ae4 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -170,7 +170,9 @@ static XSM_INLINE int cf_check xsm_domctl( case XEN_DOMCTL_bind_pt_irq: case XEN_DOMCTL_getdomaininfo: case XEN_DOMCTL_get_domain_state: + case XEN_DOMCTL_iomem_permission: case XEN_DOMCTL_ioport_mapping: + case XEN_DOMCTL_ioport_permission: case XEN_DOMCTL_memory_mapping: case XEN_DOMCTL_unbind_pt_irq: ASSERT_UNREACHABLE(); @@ -560,7 +562,7 @@ static XSM_INLINE int cf_check xsm_irq_permission( static XSM_INLINE int cf_check xsm_iomem_permission( XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow) { - XSM_ASSERT_ACTION(XSM_HOOK); + XSM_ASSERT_ACTION(XSM_PRIV); return xsm_default_action(action, current->domain, d); } @@ -756,7 +758,7 @@ static XSM_INLINE int cf_check xsm_priv_mapping( static XSM_INLINE int cf_check xsm_ioport_permission( XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow) { - XSM_ASSERT_ACTION(XSM_HOOK); + XSM_ASSERT_ACTION(XSM_PRIV); return xsm_default_action(action, current->domain, d); } diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 435cb8d661..3338a35c1e 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -653,7 +653,9 @@ static int cf_check flask_domctl(struct domain *d, unsigned int cmd, case XEN_DOMCTL_bind_pt_irq: case XEN_DOMCTL_getdomaininfo: case XEN_DOMCTL_get_domain_state: + case XEN_DOMCTL_iomem_permission: case XEN_DOMCTL_ioport_mapping: + case XEN_DOMCTL_ioport_permission: case XEN_DOMCTL_memory_mapping: case XEN_DOMCTL_unbind_pt_irq: ASSERT_UNREACHABLE(); @@ -662,14 +664,12 @@ static int cf_check flask_domctl(struct domain *d, unsigned int cmd, /* These have individual XSM hooks (common/domctl.c) */ case XEN_DOMCTL_scheduler_op: case XEN_DOMCTL_irq_permission: - case XEN_DOMCTL_iomem_permission: case XEN_DOMCTL_set_target: case XEN_DOMCTL_vm_event_op: #ifdef CONFIG_X86 /* These have individual XSM hooks (arch/x86/domctl.c) */ case XEN_DOMCTL_shadow_op: - case XEN_DOMCTL_ioport_permission: case XEN_DOMCTL_gsi_permission: #endif #ifdef CONFIG_HAS_PASSTHROUGH