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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 ).  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.

~Andrew