[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()





On 5/19/26 12:55 PM, Oleksii Kurochko wrote:


On 5/19/26 11:37 AM, Jan Beulich wrote:
On 19.05.2026 10:39, Oleksii Kurochko wrote:
vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
and falls back to dummy_vcpu_info for vcpus beyond that limit.

However, it does not guard against d->shared_info being NULL.  The
shared_info() macro expands to a member access through d->shared_info,
so when an architecture does not allocate a shared_info page the
dereference triggers UBSAN:
   UBSAN: Undefined behaviour in common/domain.c:325:10
   member access within null pointer of type 'struct shared_info_t'

Extend the existing fallback condition to also cover the case where no
shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
instead. This is the correct behaviour: dummy_vcpu_info already serves
as the safe stand-in for vcpus that have no usable shared_info slot.

Fixes: 295514ff75506 ("common: convert vCPU info area registration")

I question this, largely (but not only) because I also ...

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
---
RISC-V does not allocate a shared_info page at the momemnt because its
guests run in dom0less mode and do not use the Xen PV ABI, so
d->shared_info remains NULL throughout domain lifetime.

... question this mode of operation. Yes, you may (for now) be able to get
away without, but e.g. event channels will want supporting at some point.
Which will require a shared info page. Better put that in place right away,
even if the guests you test with don't use it (yet). Certain other common
code also assumes d->shared_info to never be NULL for an alive domain.


Would it be fine than to allocate it in arch_domain_create() ... :

     if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
         goto fail;

     clear_page(d->shared_info);

... but without calling share_xen_page_with_guest() after that allocation as share_xen_page_with_guest() isn't implemented at the moment?

Or could it be an option for all arch-s move allocation of d->shared_info to domain_create() in common just after arch_domain_create()?

The only question if share_xen_page_with_guest() could be ifdef-ed somehow so not to block new ports to implement it from the start.

~ Oleksii



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.