Re: [PATCH v2] domctl: tighten XEN_DOMCTL_*_permission

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/08/14 11:00, Andrii Tseglytskyi wrote:
> Hi,
>
> I see possible issue with this patch. Can someone clarify - did I get
> everything correctly?
>
> On Tue, May 6, 2014 at 4:08 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> @@ -790,7 +790,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>>
>>          if ( pirq >= d->nr_pirqs )
>>              ret = -EINVAL;
>> -        else if ( xsm_irq_permission(XSM_HOOK, d, pirq, allow) )
>> +        else if ( !pirq_access_permitted(current->domain, pirq) ||
> pirq_access_permitted() checks a range. Range can be added only with
> pirq_permit_access() function call. The only place where
> pirq_permit_access() is called - is following
>  *else if* branch. But it will be never called -
> pirq_access_permitted() will return 0 if range does not exist. As
> result - it is impossible to add irq, even if XSM allows this.
> The same is true for iomem_access_permitted() function call.

I questioned the same issue when this patch went in.

The argument is that, even with XSM, a domain may only permit access to
pirqs for which it also has permissions.

This prevents a buggy domain builder accidentally conferring pirq access
for a dom0 resource, without dom0 first having conferred access to the
domain builder.

~Andrew