Re: [Xen-devel] [PATCH v3 2/3] xen/pvh: check permissions when adding MMIO regions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.01.15 at 16:19, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -320,11 +320,24 @@ static __init void pvh_add_mem_mapping(struct domain 
> *d, unsigned long gfn,
>  {
>      unsigned long i;
>      p2m_access_t a;
> +    mfn_t omfn;
> +    p2m_type_t t;
>      int rc;
>  
> -    a = p2m_get_hostp2m(d)->default_access;

Iirc this is rwx.

>      for ( i = 0; i < nr_mfns; i++ )
>      {
> +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
> +        {
> +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
> +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), 
> PAGE_ORDER_4K);
> +            continue;
> +        }
> +
> +        if ( rangeset_contains_singleton(mmio_ro_ranges, mfn + i) )
> +            a = p2m_access_r;
> +        else
> +            a = p2m_access_rw;

Shouldn't these two therefore be rx and rwx respectively? Or even
better ->default_access in the else case (albeit that doesn't really
matter here since nothing can have changed that field from its
default value)? I'm particularly thinking of ROMs that may be sitting
in these areas.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel