From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arianna Avanzini Subject: [PATCH v8 14/14] xen/common: do not implicitly permit access to mapped I/O memory Date: Sun, 25 May 2014 12:51:55 +0200 Message-ID: <1401015115-7610-15-git-send-email-avanzini.arianna@gmail.com> References: <1401015115-7610-1-git-send-email-avanzini.arianna@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1401015115-7610-1-git-send-email-avanzini.arianna@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: Ian.Campbell@eu.citrix.com, paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com, tim@xen.org, julien.grall@citrix.com, etrudeau@broadcom.com, andrew.cooper3@citrix.com, JBeulich@suse.com, avanzini.arianna@gmail.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org Currently, the XEN_DOMCTL_memory_mapping hypercall implicitly grants to a domain access permission to the I/O memory areas mapped in its guest address space. This conflicts with the presence of a specific hypercall (XEN_DOMCTL_iomem_permission) used to grant such a permission to a domain. This commit separates the functions of the two hypercalls by having only the latter be able to permit I/O memory access to a domain, and the former just performing the mapping after a permissions check on both the granting and the grantee domains. Signed-off-by: Arianna Avanzini Cc: Dario Faggioli Cc: Paolo Valente Cc: Stefano Stabellini Cc: Julien Grall Cc: Ian Campbell Cc: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan Cc: Ian Jackson Cc: Andrew Cooper Cc: Eric Trudeau Cc: Viktor Kleinik --- v8: - Drop iomem_permission-related changes. - Conservatively check both the granting and the grantee domains' permissions in the memory_mapping DOMCTL. - Remove tentative phrases from commit description. v7: - Let iomem_permission check if the calling domain is allowed to access memory ranges to be mapped to a domain. Remove such a check from the memory_mapping hypercall. --- xen/common/domctl.c | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 1b246d1..bf38242 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -839,7 +839,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) break; ret = -EPERM; - if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ) + 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); @@ -852,40 +853,23 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n", d->domain_id, gfn, mfn, nr_mfns); - ret = iomem_permit_access(d, mfn, mfn_end); - if ( !ret ) - { - ret = map_mmio_regions(d, gfn, nr_mfns, _mfn(mfn)); - if ( ret ) - { - printk(XENLOG_G_WARNING - "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n", - d->domain_id, gfn, mfn, ret); - if ( iomem_deny_access(d, mfn, mfn_end) && - is_hardware_domain(current->domain) ) - printk(XENLOG_ERR - "memory_map: failed to deny dom%d access to [%lx,%lx]\n", - d->domain_id, mfn, mfn_end); - } - } + ret = map_mmio_regions(d, gfn, nr_mfns, _mfn(mfn)); + if ( ret ) + printk(XENLOG_G_WARNING + "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n", + d->domain_id, gfn, mfn, ret); } else { - int rc = 0; - printk(XENLOG_G_INFO "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n", d->domain_id, gfn, mfn, nr_mfns); - rc = unmap_mmio_regions(d, gfn, nr_mfns, _mfn(mfn)); - ret = iomem_deny_access(d, mfn, mfn_end); - if ( !ret ) - ret = rc; + ret = unmap_mmio_regions(d, gfn, nr_mfns, _mfn(mfn)); if ( ret && is_hardware_domain(current->domain) ) printk(XENLOG_ERR - "memory_map: error %ld %s dom%d access to [%lx,%lx]\n", - ret, rc ? "removing" : "denying", d->domain_id, - mfn, mfn_end); + "memory_map: error %ld removing dom%d access to [%lx,%lx]\n", + ret, d->domain_id, mfn, mfn_end); } /* Do this unconditionally to cover errors on above failure paths. */ memory_type_changed(d); -- 1.9.2