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

["Jürgen Groß" <jgross@suse.com>]

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

On 31.08.26 07:16, 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.
> 
> Call sched_destroy_vcpu() in sched_init_vcpu()'s own failure
> branch instead of sched_free_unit(), so it doesn't depend
> on the caller reaching sched_destroy_vcpu() to undo what it set
> up itself. sched_destroy_vcpu() assumes unit->priv is set, which
> is not the case here, so make it only free the udata and remove
> the unit if unit->priv in non-NULL.
> 
> Fixes: d884b1077817 ("Domain creation/destruction cleanups.")
> Signed-off-by: Furkan Caliskan <frn1furkan10@gmail.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

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

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