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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.06.2026 11:01, Oleksii Kurochko wrote:
> On 6/3/26 10:18 AM, Jan Beulich wrote:
>> On 03.06.2026 10:07, Oleksii Kurochko wrote:
>>> On 6/3/26 7:54 AM, Jan Beulich wrote:
>>>> On 02.06.2026 18:11, Oleksii Kurochko wrote:
>>>>> On 6/2/26 1:19 PM, Jan Beulich wrote:
>>>>>> On 25.05.2026 15:20, Oleksii Kurochko wrote:
>>>>> --- a/xen/common/event_fifo.c
>>>>> +++ b/xen/common/event_fifo.c
>>>>> @@ -562,9 +562,10 @@ static void setup_ports(struct domain *d, unsigned
>>>>> int prev_evtchns)
>>>>>
>>>>>             evtchn = evtchn_from_port(d, port);
>>>>>
>>>>> -        if ( d->shared_info &&
>>>>> -             guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
>>>>> +#ifdef CONFIG_HAS_SHARED_INFO
>>>>> +        if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
>>>>>                 evtchn->pending = true;
>>>>> +#endif
>>>>
>>>> While as per above shared_info() would best not exist when !HAS_SHARED_INFO
>>>> (in which case #ifdef may be unavoidable here), an alternative where
>>>> IS_ENABLED() could be used here may want at least considering. E.g.
>>>> causing a link-time failure when shared_info() is used (and not compiled
>>>> out).
>>
>> ... here. There are downsides to this, so which route to go needs settling
>> on.
> 
> For an alternative approach are you okay with the following introduction:
> 
> #ifdef CONFIG_HAS_SHARED_INFO
> #define shared_info(d, field)      __shared_info(d, (d)->shared_info, field)
> #else
> void *__shared_info_unavailable(void);
> #define shared_info(d, field) \
>      (*(typeof(__shared_info(d, (d)->shared_info, field)) 
> *)__shared_info_unavailable())
> #endif
> 
> And then use IS_ENABLED(CONFIG_HAS_SHARED_INFO) everywhere where 
> shared_info() is used including the case above:
> 
> v->vcpu_info_area.map =
>      IS_ENABLED(CONFIG_HAS_SHARED_INFO) && v->vcpu_id < XEN_LEGACY_MAX_VCPUS
>      ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
>      : &dummy_vcpu_info;
> 
> Everything that in event_2l.c could go for now without 
> IS_ENABLED(CONFIG_HAS_SHARED_INFO) where shared_info() is used as that 
> code isn't expected to be called by arch which doesn't support 2L so no 
> linkage error will occur.

Yes, this roughly is what I was thinking of. I'd like to remind you though
of issues with identifiers with two leading underscores.

Jan