Re: [PATCH v2 2/3] xen/domain: fix UBSAN null pointer dereference of d->shared_info

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.05.2026 15:20, Oleksii Kurochko wrote:
> It is legal to have d->shared_info equal to NULL for architectures which
> support only the FIFO ABI for event channel management.
> 
> Having d->shared_info == NULL leads to a UBSAN issue on such architectures:
>   UBSAN: Undefined behaviour in common/domain.c:325:10
>          member access within null pointer of type 'struct shared_info_t'
> 
> 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.
> 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.
> 
> Additionally, if an architecture supports only the FIFO ABI, setup_ports()
> should be updated to avoid a NULL pointer dereference of d->shared_info,
> since in that case there will be no pending events in
> shared_info->evtchn_pending and the pending flag of the FIFO event channel
> does not need to be set to true.
> update_domain_wallclock_time() accesses d->shared_info via shared_info()
> macro. On architectures that do not allocate a shared_info page (currently
> RISC-V, which runs guests in dom0less mode without the PV ABI), this causes
> a NULL dereference. The early return is safe: if there is no shared_info
> page, there is nothing to update. For all existing architectures (x86, ARM)
> that do allocate it, the guard is never taken and behavior is unchanged.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in v2:
>  - Update commit message + subject.
>  - Drop Fixes tag.
>  - Handle migration of pending events from 2L and FIFO ABIs when arch
>    support only FIFO ABI.

What does this item describe? On an arch supporting only FIFO, how could
evtchn need migrating from 2L?

>  - Update the commit message.
>  - Protect some other places in common code from NULL pointer deref of
>    d->shared_info.

What I'm still missing is the description clarifying why other uses don't
need guarding (or that there simply are no other uses, which - however -
I doubt).

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -320,7 +320,7 @@ void vcpu_info_reset(struct vcpu *v)
>      struct domain *d = v->domain;
>  
>      v->vcpu_info_area.map =
> -        ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
> +        ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS && d->shared_info)
>           ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
>           : &dummy_vcpu_info);
>  }

At the example of this: The extra conditionals are dead code on x86 and
Arm. While the status of the respective Misra rule is still uncertain
for Xen, imo we'd be better off avoiding the introduction of new dead
code. Which in turn means we may need some kind of abstraction to have
these extra conditionals in place only for arch-es not supporting
shared-info at all.

Jan