Re: [PATCH v4 3/3] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/05/2023 11:41 am, Jan Beulich wrote:
> On 09.05.2023 18:07, Alejandro Vallejo wrote:
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -314,7 +314,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
>> u_domctl)
>>          /* fall through */
>>      default:
>>          d = rcu_lock_domain_by_id(op->domain);
>> -        if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo )
>> +        if ( !d )
>>              return -ESRCH;
>>      }
>>  
>> @@ -534,42 +534,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
>> u_domctl)
>>  
>>      case XEN_DOMCTL_getdomaininfo:
>>      {
>> -        domid_t dom = DOMID_INVALID;
>> -
>> -        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;
>> -
>>          ret = xsm_getdomaininfo(XSM_HOOK, d);
>>          if ( ret )
>> -            goto getdomaininfo_out;
>> +            break;
>>  
>>          getdomaininfo(d, &op->u.getdomaininfo);
>>  
>>          op->domain = op->u.getdomaininfo.domain;
>>          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;
>>      }
>>  
> I realize it's a little late that this occurs to me, but this being a binary
> incompatible change it should imo have been accompanied by a bump of
> XEN_DOMCTL_INTERFACE_VERSION (which we haven't bumped yet in this release
> cycle).

Oh, sorry.  That's probably my fault.

Do you mind submitting a patch?

~Andrew