[PATCH v2 0/2] xen/sched: fix crashes when vcpu creation fails

[PATCH v2 0/2] xen/sched: fix crashes when vcpu creation fails

[Furkan Caliskan]

Furkan Caliskan (2):
  xen/sched: core: skip missing vcpu slots in sched_move_domain()
  xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

 xen/common/sched/core.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

-- 
2.34.1

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

[Furkan Caliskan]

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 <frn1furkan10@gmail.com>
---
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

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

[Jan Beulich]

On 19.08.2026 07:15, Furkan Caliskan wrote:
> --- 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] )

Is there a particular reason domain_vcpu() cannot be used here?

Jan

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

[Furkan Çalışkan]

On 8/19/26 09:53, Jan Beulich wrote:
> On 19.08.2026 07:15, Furkan Caliskan wrote:
>> --- 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] )
> 
> Is there a particular reason domain_vcpu() cannot be used here?
> 
> Jan

We still need to guard against d->max_vcpus so that out-of-bounds
indices in a partially filled unit don't get treated as missing 
vCPUs by domain_vcpu() returning NULL. 

However, domain_vcpu(unit_idx * gran + i) can be used here instead of
d->vcpu[unit_idx * gran + i]. I simply used d->vcpu[] because the rest
of the function uses it that way.

Furkan

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

[Jan Beulich]

On 19.08.2026 09:28, Furkan Çalışkan wrote:
> 
> 
> On 8/19/26 09:53, Jan Beulich wrote:
>> On 19.08.2026 07:15, Furkan Caliskan wrote:
>>> --- 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] )
>>
>> Is there a particular reason domain_vcpu() cannot be used here?
> 
> We still need to guard against d->max_vcpus so that out-of-bounds
> indices in a partially filled unit don't get treated as missing 
> vCPUs by domain_vcpu() returning NULL. 

Ah, right - the bounds check cannot really be folded here. Then ...

> However, domain_vcpu(unit_idx * gran + i) can be used here instead of
> d->vcpu[unit_idx * gran + i]. I simply used d->vcpu[] because the rest
> of the function uses it that way.

... best wait for Jürgen to comment. Outside of the scheduler we're
trying to replace open-coding of domain_vcpu(), but inside the
scheduler things may be different.

Jan

[PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Furkan Caliskan]

sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
singleshot_timer and poll_timer before it can fail -- these
become live, linked into their target pCPU's per-cpu timer list
regardless of what happens next. If the sched_alloc_udata() call
further down then fails, the function frees the sched_unit via
sched_free_unit() and returns 1, but never unlinks these three
timers.

The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
returning nonzero it jumps to fail_wq, skipping fail_sched and
thus sched_destroy_vcpu() -- the only function on this path that
calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
and the three timers embedded in it, while they are still linked
into that shared list.

This silently corrupts that list. It only shows up later, when
something else touches a neighboring timer: sched_move_domain()
crashed with "Assertion 'entry->prev->next == entry' failed" on a
completely unrelated, valid vcpu's timer.

Kill all three timers in sched_init_vcpu()'s own failure branch,
so it doesn't depend on the caller reaching sched_destroy_vcpu()
to undo what it set up itself.

Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")
Signed-off-by: Furkan Caliskan <frn1furkan10@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
v2:
 - Added Fixes: tag.
---
 xen/common/sched/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index a9daa42339..5777096592 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
     unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
     if ( unit->priv == NULL )
     {
+        kill_timer(&v->periodic_timer);
+        kill_timer(&v->singleshot_timer);
+        kill_timer(&v->poll_timer);
         sched_free_unit(unit, v);
         rcu_read_unlock(&sched_res_rculock);
         return 1;
-- 
2.34.1

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Jan Beulich]

On 19.08.2026 07:15, Furkan Caliskan wrote:
> sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
> singleshot_timer and poll_timer before it can fail -- these
> become live, linked into their target pCPU's per-cpu timer list
> regardless of what happens next. If the sched_alloc_udata() call
> further down then fails, the function frees the sched_unit via
> sched_free_unit() and returns 1, but never unlinks these three
> timers.
> 
> The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
> returning nonzero it jumps to fail_wq, skipping fail_sched and
> thus sched_destroy_vcpu() -- the only function on this path that
> calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
> and the three timers embedded in it, while they are still linked
> into that shared list.
> 
> This silently corrupts that list. It only shows up later, when
> something else touches a neighboring timer: sched_move_domain()
> crashed with "Assertion 'entry->prev->next == entry' failed" on a
> completely unrelated, valid vcpu's timer.
> 
> Kill all three timers in sched_init_vcpu()'s own failure branch,
> so it doesn't depend on the caller reaching sched_destroy_vcpu()
> to undo what it set up itself.
> 
> Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")

How did you arrive at this commit? It doesn't even touch sched_init_vcpu().
All it does is move kill_timer() invocations around. I think it's
d884b1077817, as that's where the "return SCHED_OP(init_vcpu, v)" was
introduced (i.e. where kill_timer() would have been necessary to call in
the error case). (I can't exclude the issue was pre-existing already at
that time, but that would require more analysis than I think is worth to
invest.)

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>      if ( unit->priv == NULL )
>      {
> +        kill_timer(&v->periodic_timer);
> +        kill_timer(&v->singleshot_timer);
> +        kill_timer(&v->poll_timer);
>          sched_free_unit(unit, v);
>          rcu_read_unlock(&sched_res_rculock);
>          return 1;

This almost, but not quite open-codes sched_destroy_vcpu(). Would be nice
if the cleanup logic was shared. The sched_free_unit() call there could be
leveraged here as well; what would need skipping are the sched_free_udata()
and sched_remove_unit(). And of course the RCU-locking would need sorting.

Jan

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Furkan Çalışkan]

On 8/19/26 10:22, Jan Beulich wrote:
> On 19.08.2026 07:15, Furkan Caliskan wrote:
>> sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
>> singleshot_timer and poll_timer before it can fail -- these
>> become live, linked into their target pCPU's per-cpu timer list
>> regardless of what happens next. If the sched_alloc_udata() call
>> further down then fails, the function frees the sched_unit via
>> sched_free_unit() and returns 1, but never unlinks these three
>> timers.
>>
>> The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
>> returning nonzero it jumps to fail_wq, skipping fail_sched and
>> thus sched_destroy_vcpu() -- the only function on this path that
>> calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
>> and the three timers embedded in it, while they are still linked
>> into that shared list.
>>
>> This silently corrupts that list. It only shows up later, when
>> something else touches a neighboring timer: sched_move_domain()
>> crashed with "Assertion 'entry->prev->next == entry' failed" on a
>> completely unrelated, valid vcpu's timer.
>>
>> Kill all three timers in sched_init_vcpu()'s own failure branch,
>> so it doesn't depend on the caller reaching sched_destroy_vcpu()
>> to undo what it set up itself.
>>
>> Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")
> 
> How did you arrive at this commit? It doesn't even touch sched_init_vcpu().
> All it does is move kill_timer() invocations around. I think it's
> d884b1077817, as that's where the "return SCHED_OP(init_vcpu, v)" was
> introduced (i.e. where kill_timer() would have been necessary to call in
> the error case). (I can't exclude the issue was pre-existing already at
> that time, but that would require more analysis than I think is worth to
> invest.)

Commit 1ad5dad74cde moved kill_timer() calls into sched_destroy_vcpu() 
function, which is not called if sched_init_vcpu() fails. 
Before that commit, kill_timer() calls were in sched_destroy_domain(), 
which is called if sched_init_vcpu() returns non-zero to its caller, 
alloc_vcpu(). 

        for ( i = 0; i < max; i++ )
        {
            if ( d->vcpu[i] != NULL )
                continue;

            cpu = (i == 0) ?
                default_vcpu0_location() :
                (d->vcpu[i-1]->processor + 1) % num_online_cpus();

            if ( alloc_vcpu(d, i, cpu) == NULL )
                goto maxvcpu_out;
        }

        ret = 0;

    maxvcpu_out:
        domain_unpause(d);
        put_domain(d);
    }
    break;

put_domain() calls domain_destroy(), which then calls free_domain(), 
which ultimately calls sched_destroy_domain().

> 
>> --- a/xen/common/sched/core.c
>> +++ b/xen/common/sched/core.c
>> @@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
>>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>>      if ( unit->priv == NULL )
>>      {
>> +        kill_timer(&v->periodic_timer);
>> +        kill_timer(&v->singleshot_timer);
>> +        kill_timer(&v->poll_timer);
>>          sched_free_unit(unit, v);
>>          rcu_read_unlock(&sched_res_rculock);
>>          return 1;
> 
> This almost, but not quite open-codes sched_destroy_vcpu(). Would be nice
> if the cleanup logic was shared. The sched_free_unit() call there could be
> leveraged here as well; what would need skipping are the sched_free_udata()
> and sched_remove_unit(). And of course the RCU-locking would need sorting.
> 
> Jan

Furkan

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Jan Beulich]

On 19.08.2026 09:53, Furkan Çalışkan wrote:
> 
> 
> On 8/19/26 10:22, Jan Beulich wrote:
>> On 19.08.2026 07:15, Furkan Caliskan wrote:
>>> sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
>>> singleshot_timer and poll_timer before it can fail -- these
>>> become live, linked into their target pCPU's per-cpu timer list
>>> regardless of what happens next. If the sched_alloc_udata() call
>>> further down then fails, the function frees the sched_unit via
>>> sched_free_unit() and returns 1, but never unlinks these three
>>> timers.
>>>
>>> The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
>>> returning nonzero it jumps to fail_wq, skipping fail_sched and
>>> thus sched_destroy_vcpu() -- the only function on this path that
>>> calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
>>> and the three timers embedded in it, while they are still linked
>>> into that shared list.
>>>
>>> This silently corrupts that list. It only shows up later, when
>>> something else touches a neighboring timer: sched_move_domain()
>>> crashed with "Assertion 'entry->prev->next == entry' failed" on a
>>> completely unrelated, valid vcpu's timer.
>>>
>>> Kill all three timers in sched_init_vcpu()'s own failure branch,
>>> so it doesn't depend on the caller reaching sched_destroy_vcpu()
>>> to undo what it set up itself.
>>>
>>> Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")
>>
>> How did you arrive at this commit? It doesn't even touch sched_init_vcpu().
>> All it does is move kill_timer() invocations around. I think it's
>> d884b1077817, as that's where the "return SCHED_OP(init_vcpu, v)" was
>> introduced (i.e. where kill_timer() would have been necessary to call in
>> the error case). (I can't exclude the issue was pre-existing already at
>> that time, but that would require more analysis than I think is worth to
>> invest.)
> 
> Commit 1ad5dad74cde moved kill_timer() calls into sched_destroy_vcpu() 
> function, which is not called if sched_init_vcpu() fails. 
> Before that commit, kill_timer() calls were in sched_destroy_domain(), 
> which is called if sched_init_vcpu() returns non-zero to its caller, 
> alloc_vcpu(). 
> 
>         for ( i = 0; i < max; i++ )
>         {
>             if ( d->vcpu[i] != NULL )
>                 continue;
> 
>             cpu = (i == 0) ?
>                 default_vcpu0_location() :
>                 (d->vcpu[i-1]->processor + 1) % num_online_cpus();
> 
>             if ( alloc_vcpu(d, i, cpu) == NULL )
>                 goto maxvcpu_out;
>         }
> 
>         ret = 0;
> 
>     maxvcpu_out:
>         domain_unpause(d);
>         put_domain(d);
>     }
>     break;
> 
> put_domain() calls domain_destroy(), which then calls free_domain(), 
> which ultimately calls sched_destroy_domain().

Well, yes, except that - how would that have helped for a vCPU which
failed to be properly constructed? The function loops over all vCPU-s
in the domain, but that wouldn't include the vCPU in question.
alloc_vcpu() would (of course) insert the vCPU into the list only when
sched_init_vcpu() succeeds.

Jan

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Furkan Çalışkan]

On 8/19/26 11:32, Jan Beulich wrote:
> On 19.08.2026 09:53, Furkan Çalışkan wrote:
>>
>>
>> On 8/19/26 10:22, Jan Beulich wrote:
>>> On 19.08.2026 07:15, Furkan Caliskan wrote:
>>>> sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
>>>> singleshot_timer and poll_timer before it can fail -- these
>>>> become live, linked into their target pCPU's per-cpu timer list
>>>> regardless of what happens next. If the sched_alloc_udata() call
>>>> further down then fails, the function frees the sched_unit via
>>>> sched_free_unit() and returns 1, but never unlinks these three
>>>> timers.
>>>>
>>>> The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
>>>> returning nonzero it jumps to fail_wq, skipping fail_sched and
>>>> thus sched_destroy_vcpu() -- the only function on this path that
>>>> calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
>>>> and the three timers embedded in it, while they are still linked
>>>> into that shared list.
>>>>
>>>> This silently corrupts that list. It only shows up later, when
>>>> something else touches a neighboring timer: sched_move_domain()
>>>> crashed with "Assertion 'entry->prev->next == entry' failed" on a
>>>> completely unrelated, valid vcpu's timer.
>>>>
>>>> Kill all three timers in sched_init_vcpu()'s own failure branch,
>>>> so it doesn't depend on the caller reaching sched_destroy_vcpu()
>>>> to undo what it set up itself.
>>>>
>>>> Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")
>>>
>>> How did you arrive at this commit? It doesn't even touch sched_init_vcpu().
>>> All it does is move kill_timer() invocations around. I think it's
>>> d884b1077817, as that's where the "return SCHED_OP(init_vcpu, v)" was
>>> introduced (i.e. where kill_timer() would have been necessary to call in
>>> the error case). (I can't exclude the issue was pre-existing already at
>>> that time, but that would require more analysis than I think is worth to
>>> invest.)
>>
>> Commit 1ad5dad74cde moved kill_timer() calls into sched_destroy_vcpu() 
>> function, which is not called if sched_init_vcpu() fails. 
>> Before that commit, kill_timer() calls were in sched_destroy_domain(), 
>> which is called if sched_init_vcpu() returns non-zero to its caller, 
>> alloc_vcpu(). 
>>
>>         for ( i = 0; i < max; i++ )
>>         {
>>             if ( d->vcpu[i] != NULL )
>>                 continue;
>>
>>             cpu = (i == 0) ?
>>                 default_vcpu0_location() :
>>                 (d->vcpu[i-1]->processor + 1) % num_online_cpus();
>>
>>             if ( alloc_vcpu(d, i, cpu) == NULL )
>>                 goto maxvcpu_out;
>>         }
>>
>>         ret = 0;
>>
>>     maxvcpu_out:
>>         domain_unpause(d);
>>         put_domain(d);
>>     }
>>     break;
>>
>> put_domain() calls domain_destroy(), which then calls free_domain(), 
>> which ultimately calls sched_destroy_domain().
> 
> Well, yes, except that - how would that have helped for a vCPU which
> failed to be properly constructed? The function loops over all vCPU-s
> in the domain, but that wouldn't include the vCPU in question.
> alloc_vcpu() would (of course) insert the vCPU into the list only when
> sched_init_vcpu() succeeds.
> 
> Jan

Ah, I missed that compeletely - you're right.
sched_destroy_domain() wouldn't have reached the failed vCPU anyway.

In that case, d884b1077817 makes total sense here.

Furkan

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Furkan Çalışkan]

On 8/19/26 10:22, Jan Beulich wrote:
> On 19.08.2026 07:15, Furkan Caliskan wrote:
>> sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
>> singleshot_timer and poll_timer before it can fail -- these
>> become live, linked into their target pCPU's per-cpu timer list
>> regardless of what happens next. If the sched_alloc_udata() call
>> further down then fails, the function frees the sched_unit via
>> sched_free_unit() and returns 1, but never unlinks these three
>> timers.
>>
>> The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
>> returning nonzero it jumps to fail_wq, skipping fail_sched and
>> thus sched_destroy_vcpu() -- the only function on this path that
>> calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
>> and the three timers embedded in it, while they are still linked
>> into that shared list.
>>
>> This silently corrupts that list. It only shows up later, when
>> something else touches a neighboring timer: sched_move_domain()
>> crashed with "Assertion 'entry->prev->next == entry' failed" on a
>> completely unrelated, valid vcpu's timer.
>>
>> Kill all three timers in sched_init_vcpu()'s own failure branch,
>> so it doesn't depend on the caller reaching sched_destroy_vcpu()
>> to undo what it set up itself.
>>
>> Fixes: 1ad5dad74cde ("[XEN] Re-jig VCPU initialisation -- VMX init requires generic VCPU")
> 
> How did you arrive at this commit? It doesn't even touch sched_init_vcpu().
> All it does is move kill_timer() invocations around. I think it's
> d884b1077817, as that's where the "return SCHED_OP(init_vcpu, v)" was
> introduced (i.e. where kill_timer() would have been necessary to call in
> the error case). (I can't exclude the issue was pre-existing already at
> that time, but that would require more analysis than I think is worth to
> invest.)
> 
>> --- a/xen/common/sched/core.c
>> +++ b/xen/common/sched/core.c
>> @@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
>>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>>      if ( unit->priv == NULL )
>>      {
>> +        kill_timer(&v->periodic_timer);
>> +        kill_timer(&v->singleshot_timer);
>> +        kill_timer(&v->poll_timer);
>>          sched_free_unit(unit, v);
>>          rcu_read_unlock(&sched_res_rculock);
>>          return 1;
> 
> This almost, but not quite open-codes sched_destroy_vcpu(). Would be nice
> if the cleanup logic was shared. The sched_free_unit() call there could be
> leveraged here as well; what would need skipping are the sched_free_udata()
> and sched_remove_unit(). And of course the RCU-locking would need sorting.
> 
> Jan

Would something like below be okay?

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index d3a0a97e1d..36704a5836 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -589,8 +589,8 @@ int sched_init_vcpu(struct vcpu *v)
     unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
     if ( unit->priv == NULL )
     {
-        sched_free_unit(unit, v);
         rcu_read_unlock(&sched_res_rculock);
+        sched_destroy_vcpu(v);
         return 1;
     }
 
@@ -869,8 +869,11 @@ void sched_destroy_vcpu(struct vcpu *v)
     {
         rcu_read_lock(&sched_res_rculock);
 
-        sched_remove_unit(vcpu_scheduler(v), unit);
-        sched_free_udata(vcpu_scheduler(v), unit->priv);
+        if ( unit->priv )
+        {
+            sched_remove_unit(vcpu_scheduler(v), unit);
+            sched_free_udata(vcpu_scheduler(v), unit->priv);
+        }
         sched_free_unit(unit, v);
 
         rcu_read_unlock(&sched_res_rculock);


Furkan

Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Jan Beulich]

On 19.08.2026 12:39, Furkan Çalışkan wrote:
> On 8/19/26 10:22, Jan Beulich wrote:
>> On 19.08.2026 07:15, Furkan Caliskan wrote:
>>> --- a/xen/common/sched/core.c
>>> +++ b/xen/common/sched/core.c
>>> @@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
>>>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>>>      if ( unit->priv == NULL )
>>>      {
>>> +        kill_timer(&v->periodic_timer);
>>> +        kill_timer(&v->singleshot_timer);
>>> +        kill_timer(&v->poll_timer);
>>>          sched_free_unit(unit, v);
>>>          rcu_read_unlock(&sched_res_rculock);
>>>          return 1;
>>
>> This almost, but not quite open-codes sched_destroy_vcpu(). Would be nice
>> if the cleanup logic was shared. The sched_free_unit() call there could be
>> leveraged here as well; what would need skipping are the sched_free_udata()
>> and sched_remove_unit(). And of course the RCU-locking would need sorting.
> 
> Would something like below be okay?

Maybe, but you need to ask the maintainers of this code, which I'm not a part
of. What I in particular can't easily judge is whether ...

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -589,8 +589,8 @@ int sched_init_vcpu(struct vcpu *v)
>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>      if ( unit->priv == NULL )
>      {
> -        sched_free_unit(unit, v);
>          rcu_read_unlock(&sched_res_rculock);
> +        sched_destroy_vcpu(v);
>          return 1;
>      }

... this intermediate dropping of the lock is entirely okay (it looks to be
at the first glance).

Jan

> @@ -869,8 +869,11 @@ void sched_destroy_vcpu(struct vcpu *v)
>      {
>          rcu_read_lock(&sched_res_rculock);
>  
> -        sched_remove_unit(vcpu_scheduler(v), unit);
> -        sched_free_udata(vcpu_scheduler(v), unit->priv);
> +        if ( unit->priv )
> +        {
> +            sched_remove_unit(vcpu_scheduler(v), unit);
> +            sched_free_udata(vcpu_scheduler(v), unit->priv);
> +        }
>          sched_free_unit(unit, v);
>  
>          rcu_read_unlock(&sched_res_rculock);
> 
> 
> Furkan