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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2026 13:22, Oleksii Kurochko wrote:
> 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?

I would have said "yes" here, but ...

> 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()?

... Andrew's reply pretty much rules out not only this option, but the
shared-info-page concept as a whole (for RISC-V). See my reply there. In
the meantime, the change as suggested may then indeed be what we want to
go with, albeit (a) with a better description and (b) perhaps covering
all d->shared_info uses.

Jan