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