Re: [PATCH v3 1/2] xen/common: add vcpus_create() and keep max_vcpus in sync
Andrew Cooper <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <[email protected]> |
On 31/08/2026 6:16 am, Furkan Caliskan wrote:
> Every vcpu_create() call site that builds more than one vcpu loops
> over ids up to d->max_vcpus and stops on the first failure, but none
> of them roll max_vcpus back to match. This leaves d->vcpu[i] == NULL
> for ids below max_vcpus
As I told you before, you must cope with this property in non-error
scenarios.
> , which anything walking d->vcpu[] can then
> dereference. This is what caused the crash: sched_move_domain()
> walks every vcpu slot up to max_vcpus without checking for empty
> ones, so when a domain built in a non-default cpupool had vcpu
> creation fail partway through, domain_kill() later moving it back
> to the default cpupool handed one of its empty slots straight to
> the new cpupool's scheduler, causing a NULL-pointer dereference
> inside sched_alloc_udata().
>
> Add vcpus_create(d): creates every vcpu of d up to max_vcpus and
> rolls max_vcpus back to the failed id on error. This keeps
> d->vcpu[i] is non-NULL for all i < d->max_vcpus
No, it really doesn't.
> , instead of guarding
> every reader of d->vcpu[] agains holes individually.
>
> Convert every site that builds vcpus in a loop to call this function
> instead.
>
> Fixes: 61649709421a ("xen/domain: Allocate d->vcpu[] in domain_create()")
> Suggested-by: Juergen Gross <[email protected]>
> Signed-off-by: Furkan Caliskan <[email protected]>
For the avoidance of a long drawn-out argument, nack. Under no
circumstances are you editing d->max_cpus after it's put into the domain
list.
You've chosen to do so at a point where the domain object is live,
visible in the system and able to be the target of other hypercalls.
Furthermore you have not fixed what your commit message claims.
d->vcpu[...] is still NULL for an arbitrary period of time, including
being able to be the target of hypercalls, before vCPUs are created.
All code MUST be able to cope with d->vcpu[...] being NULL. It's how
the object lifecycles must work, because creating vCPUs is not atomic
with respect to creating domains.
~Andrew