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 11:03, <JBeulich@xxxxxxxx> wrote:
>>>> On 25.05.14 at 19:14, <julien.grall@xxxxxxxxxx> wrote:
>> On 06/05/14 10:06, Jan Beulich wrote:
>>>> @@ -838,7 +842,7 @@ 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(d, mfn, mfn_end) )
>>>
>>> I'm not sure if we shouldn't rather be conservative here and check
>>> both domains' permissions.
>> 
>> I think we only need to check the permission of the current domain. 
>> Checking the permission of the guest is not necessary here. Hence, the 
>> code to map a region will be more difficult. We will have first call 
>> iomem_permission then call memory_mapping.
>> 
>> I would prefer to keep the current permission check ie:
>> 
>> if ( !iomem_access_permitted(current->domain, ...) )
> 
> In fact my earlier comment was wrong - for some reason I was
> thinking this was the mapping operation that gets modified here,
> not the permission one. For the permission one it is of course
> wrong to check the subject domain's permissions, as they're only
> about to be granted.

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

Jan


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