Re: [PATCH v2 01/21] xen/common: do not implicitly permit access to mapped I/O memory

[Julien Grall <julien.grall@linaro.org>]

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);
> 


-- 
Julien Grall