Re: [PATCH v5 3/4] xen/console: remove max_init_domid dependency

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.06.2025 02:36, Stefano Stabellini wrote:
> On Fri, 30 May 2025, dmkhn@xxxxxxxxx wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -2461,6 +2461,39 @@ void domid_free(domid_t domid)
>>      spin_unlock(&domid_lock);
>>  }
>>  
>> +/*
>> + * Find the ID of the next possible console owner domain.
>> + *
>> + * @return Domain ID: DOMID_XEN or non-system domain IDs within
>> + * the range of [0..DOMID_FIRST_RESERVED-1].
>> + */
>> +domid_t domid_find_with_input_allowed(domid_t hint)
>> +{
>> +    domid_t domid = DOMID_XEN;
>> +
>> +    if ( hint < DOMID_FIRST_RESERVED )
>> +    {
>> +        struct domain *d;
>> +
>> +        rcu_read_lock(&domlist_read_lock);
>> +
>> +        for ( d = domid_to_domain(hint);
>> +              d && get_domain(d) && d->domain_id < DOMID_FIRST_RESERVED;
> 
> The get_domain(d) worries me because it is increasing the domain's
> refcnt but I don't see a corresponding call to put_domain to decrease
> it.
> 
> If I keep rotating between consoles, I could keep increasing refcnt
> indefinitely?
> 
> I think we either need a corresponding put_domain(d) call when the domain
> loses input focus, or we remove the get_domain(d) based on the fact that
> we don't need it. I think before this patch we didn't increment refcnt
> when a domain has focus but I am not sure it was correct.

I think it was. The code was - aiui - specifically prepared to deal with
domains going away behind its back. A domain having input focus should
not prevent it from being (fully) destroyed.

Jan