diff mbox

[Xen-devel,v2,01/21] xen/common: do not implicitly permit access to mapped I/O memory

Message ID 1406818852-31856-2-git-send-email-julien.grall@linaro.org
State Accepted
Commit 0561e1f01e87b777bcc47971e4ae1f420314f4a0
Headers show

Commit Message

Julien Grall July 31, 2014, 3 p.m. UTC
From: Arianna Avanzini <avanzini.arianna@gmail.com>

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 <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
---
 xen/common/domctl.c |   36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

Comments

Julien Grall July 31, 2014, 3:22 p.m. UTC | #1
Hi all,

Please forgot this patch. I've added it by mistake in my series.

Regards,

On 07/31/2014 04:00 PM, Julien Grall wrote:
> From: Arianna Avanzini <avanzini.arianna@gmail.com>
> 
> 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 <avanzini.arianna@gmail.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Julien Grall <julien.grall@citrix.com>
> Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Eric Trudeau <etrudeau@broadcom.com>
> Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
> ---
>  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 80b7800..04ecd53 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -917,7 +917,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);
> @@ -930,40 +931,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);
> -                if ( ret )
> -                {
> -                    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);
> -                    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);
> +            if ( ret )
> +                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
>          {
> -            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);
> -            ret = iomem_deny_access(d, mfn, mfn_end);
> -            if ( !ret )
> -                ret = rc;
> +            ret = unmap_mmio_regions(d, gfn, nr_mfns, 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);
>
diff mbox

Patch

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 80b7800..04ecd53 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -917,7 +917,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);
@@ -930,40 +931,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);
-                if ( ret )
-                {
-                    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);
-                    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);
+            if ( ret )
+                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
         {
-            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);
-            ret = iomem_deny_access(d, mfn, mfn_end);
-            if ( !ret )
-                ret = rc;
+            ret = unmap_mmio_regions(d, gfn, nr_mfns, 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);