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:56, Andrew Cooper wrote:
> On 19/05/2026 12:51 pm, Jan Beulich wrote:
>> On 19.05.2026 13:32, Andrew Cooper wrote:
>>> 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.
>> In which case, how do you propose e.g. event channels to be handled in
>> whatever is going to be the alternative?
> 
> Implement proper enumeration of virtual capabilities (to be retrofitted
> to x86/ARM too), and only offer the FIFO ABI (which is superior in every
> way to the 2L ABI).

What about the wc_* fields then? And about everything in struct 
arch_shared_info?

Jan