Re: [Xen-devel] [PATCH v7 10/10] xen/common: do not implicitly permit access to mapped I/O memory

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.05.14 at 13:24, <julien.grall@xxxxxxxxxx> wrote:
> On 26/05/14 12:14, Jan Beulich wrote:
>>>>> On 26.05.14 at 12:53, <julien.grall@xxxxxxxxxx> wrote:
>>> On 26/05/14 11:14, Jan Beulich wrote:
>>>>
>>>> Or maybe I wasn't wrong - the patch context doesn't really make
>>>> clear whether it's the granting or mapping operation that gets
>>>> adjusted here (since an earlier patch moved the mapping one into
>>>> this function).
>>>
>>>            ret = -EPERM;
>>> -        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
>>> +        if ( !iomem_access_permitted(d, mfn, mfn_end) )
>>>                break;
>>>
>>>            ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
>>>
>>> There is an xsm_iomem_mapping just after, so the change has been done in
>>> XEN_DOMCTL_memory_mapping.
>>
>> In which case I indeed stick to my original comment - it's perhaps
>> best to check _both_.
> 
> Why? We may want to map the region in the guest P2M without giving the 
> permission to the guest (I'm thinking about ARM passthrough case).

How can you put a mapping of memory into a guest's P2M for which
that guest has no access permission? To me this reads like you're
intending to create a security issue here.

> With your requirements, we have to call 2 hypercalls rather than one for 
> memory mapping, even if we don't want to allow the guest modifying iomem 
> range.

While I can see you not allowing modification, even r/o access may
(and likely will) be problematic for MMIO.

Jan


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