[PATCH v2 1/2] xen/sched: core: skip missing vcpu slots in sched_move_domain()

Furkan Caliskan <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
sched_move_domain() derives the number of units to rebuild from
d->max_vcpus, which is fixed at domain creation and never rolled
back if vcpu_create() fails partway through building a domain. So
d->vcpu[i] can be NULL for some i even though max_vcpus still
counts it - this happens if sched_alloc_udata() returns NULL.

The per-unit loop doesn't check for this: it sets
unit->vcpu_list = d->vcpu[unit_id] (NULL) and hands that broken
unit straight to the destination scheduler's alloc_udata(),
which assumes vcpu_list is always valid and crashes Xen when
it is not.

Reproduced by building a domain in a non-default cpupool where
vcpu creation fails partway through, then destroying it.
domain_kill() moves the domain back to the default cpupool via
sched_move_domain() before actually destroying it, crashing
inside the destination scheduler's alloc_udata() (seen in
Credit2's csched2_alloc_udata() -> is_idle_unit() -> NULL deref).

Before building a unit in sched_move_domain(), check whether all
vpcu slots belonging to that unit are populated. If any of its
vpcus is missing:
 - For a dying domain, skip the unit allocation.
 - For an active domain, abort the move and return -EINVAL to
   prevent running with dropped vCPUs.

Fixes: 70fadc41635b ("xen/cpupool: support moving domain between cpupools with different granularity")
Signed-off-by: Furkan Caliskan <[email protected]>
---
v2:
 - Fail with -EINVAL if vcpu slots are missing in an active domain.
 - Added Fixes: tag.
---
 xen/common/sched/core.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index d3a0a97e1d..a9daa42339 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -745,6 +745,38 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
 
     for ( unit_idx = 0; unit_idx < n_units; unit_idx++ )
     {
+        /*
+         * A vcpu slot can be missing if creation failed partway
+         * through. A dying domain is being torn down regardless, so
+         * skip the unit -- but a domain that isn't dying still needs
+         * every vcpu it has schedulable, so fail instead of silently
+         * dropping some of them.
+         */
+        bool vcpu_failed = false;
+
+        for ( unsigned int i = 0;
+              i < gran && unit_idx * gran + i < d->max_vcpus; i++ )
+        {
+            if ( !d->vcpu[unit_idx * gran + i] )
+            {
+                vcpu_failed = true;
+                break;
+            }
+        }
+
+        if ( vcpu_failed )
+        {
+            if ( !d->is_dying )
+            {
+                sched_move_domain_cleanup(c->sched, new_units, domdata);
+                rcu_read_unlock(&sched_res_rculock);
+
+                return -EINVAL;
+            }
+
+            continue;
+        }
+
         unit = sched_alloc_unit_mem();
         if ( unit )
         {
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.