Re: [PATCH] xen/sched: fix sched_move_domain() for domain without vcpus

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1858 bytes --]

On 06.09.21 13:14, Andrew Cooper wrote:
> On 06/09/2021 12:00, Juergen Gross wrote:
>> In case a domain is created with a cpupool other than Pool-0 specified
>> it will be moved to that cpupool before any vcpus are allocated.
>>
>> This will lead to a NULL pointer dereference in sched_move_domain().
>>
>> Fix that by tolerating vcpus not being allocated yet.
>>
>> Fixes: 70fadc41635b9b6 ("xen/cpupool: support moving domain between cpupools with different granularity")
>> Reported-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   xen/common/sched/core.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
>> index 8d178baf3d..79c9100680 100644
>> --- a/xen/common/sched/core.c
>> +++ b/xen/common/sched/core.c
>> @@ -671,6 +671,10 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
>>   
>>       for ( unit_idx = 0; unit_idx < n_units; unit_idx++ )
>>       {
>> +        /* Special case for move at domain creation time. */
>> +        if ( !d->vcpu[unit_idx * gran] )
>> +            break;
>> +
>>           unit = sched_alloc_unit_mem();
>>           if ( unit )
>>           {
> 
> I think the logic would be clearer if you wrap the entire for loop in if
> ( d->max_vcpus ).

No, d->max_vcpus is not 0 here, otherwise n_units would be 0.

> This loop is only allocating units in the new
> scheduler for existing vcpus, so there's no point entering the loop at
> all during domain creation.
> 
> Also, this removes a non-speculatively-guarded d->vcpu[] deference.

I don't think this dereference is a real problem. In case you are
worried about it we should replace the one further below, too.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]