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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.04.14 at 19:17, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 30/04/14 15:24, Jan Beulich wrote:
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -72,13 +72,11 @@ long arch_do_domctl(
>>          unsigned int np = domctl->u.ioport_permission.nr_ports;
>>          int allow = domctl->u.ioport_permission.allow_access;
>>  
>> -        ret = -EINVAL;
>> -        if ( (fp + np) > 65536 )
>> -            break;
>> -
>> -        if ( np == 0 )
>> -            ret = 0;
>> -        else if ( xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, 
>> allow) 
> )
>> +        if ( (fp + np - 1) < fp || (fp + np) > 0x10000 )
>> +            ret = -EINVAL;
>> +        else if ( !ioports_access_permitted(current->domain,
>> +                                            fp, fp + np - 1) ||
>> +                  xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, 
>> allow) )
> 
> I don't see what the ioport permissions of the current domain have to do
> with whether a domain is permitted to change the permissions for another
> domain.
> 
> I would expect that any domain builder domains would have no ioport
> permissions at all, which would cause this hypercall to unconditionally
> fail with -EPERM even if the domain builder domain is permitted to
> assign ioport permissions to the domain it is building.

My perspective is quite different - a building/controlling domain shouldn't
be able to assign any resources it doesn't itself have control over. Which
matches up with XEN_DOMCTL_{memory,ioport}_mapping, which too
check requester permissions before doing the permission assignment
(albeit there seems to be a tendency to agreement that the combination
of permission granting and mapping is undesirable).

Jan


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