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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.08.16 at 17:55, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

How about:

@@ -839,6 +846,10 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         copyback = 1;
 
     getdomaininfo_out:
+        /* When d was non-NULL upon entry, no cleanup is needed. */
+        if ( dom == DOMID_INVALID )
+            break;
+
         rcu_read_unlock(&domlist_read_lock);
         d = NULL;
         break;

Jan


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