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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/05/2026 12:22 pm, 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?
>
> 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.

shared_info is an x86-PV-ism which escaped into HVM and then infected
ARM too.

Sadly it's ABI there, but this is one of many areas where I really want
RISC-V not to inherit the mistakes of prior ports.

~Andrew