Re: [Xen-devel] [PATCH] domctl: relax getdomaininfo permissions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/08/16 16:44, Jan Beulich wrote:
>>>> On 04.08.16 at 17:22, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 04/08/16 09:41, Jan Beulich wrote:
>>> @@ -817,14 +816,22 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>>>  
>>>      case XEN_DOMCTL_getdomaininfo:
>>>      {
>>> -        domid_t dom = op->domain;
>>> -
>>> -        rcu_read_lock(&domlist_read_lock);
>>> +        domid_t dom = DOMID_INVALID;
>>>  
>>> -        for_each_domain ( d )
>>> -            if ( d->domain_id >= dom )
>>> +        if ( !d )
>>> +        {
>>> +            ret = -EINVAL;
>>> +            if ( op->domain >= DOMID_FIRST_RESERVED )
>>>                  break;
>>>  
>>> +            rcu_read_lock(&domlist_read_lock);
>>> +
>>> +            dom = op->domain;
>>> +            for_each_domain ( d )
>>> +                if ( d->domain_id >= dom )
>>> +                    break;
>>> +        }
>>> +
>>>          ret = -ESRCH;
>>>          if ( d == NULL )
>>>              goto getdomaininfo_out;
>>> @@ -839,6 +846,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>>>          copyback = 1;
>>>  
>>>      getdomaininfo_out:
>>> +        if ( dom == DOMID_INVALID )
>>> +            break;
>> What is this hunk for?  If you fail the "op->domain >=
>> DOMID_FIRST_RESERVED" check we break out of the entire
>> XEN_DOMCTL_getdomaininfo case.
> If we start out with a non-NULL d, we have to avoid the
> rcu_read_unlock() as well as the setting of d to NULL at the
> end.

Would you mind adding a short comment to that effect?  It is certainly
not obvious from just looking at the code.

~Andrew

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