[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] domctl: tighten XEN_DOMCTL_*_permission

Hi Andrew,

Thank you for your comment.

On Fri, Aug 15, 2014 at 1:36 PM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
> 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@xxxxxxxx> 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.

Okay. This sounds reasonable.
One more question - I see that pirq_access_permitted() calls with
current->domain pointer, which points to dom0.
So, if resource belongs to dom0 - it must not belong to any other
domain. But in current implementation pirq_access_permitted() returns
0 if resource is not found.
In other words - resource does not belong to dom0, but -EPERM error
will be returned anyway.


> ~Andrew


Andrii Tseglytskyi | Embedded Dev

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.