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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 6/3/26 7:54 AM, Jan Beulich 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).
We still want here to have #ifdef instead of IS_ENABLED() as 
shared_info() shouldn't exist for arch without 2L support so it will end 
with linkage error. Considering that setup_ports() will be called for 
such arch we have to avoid this part from compilation.
Alternative is that considering that I suggested in prev emails to 
introduced stubs for arch which doesn't use 2L:
+#ifndef CONFIG_HAS_SHARED_INFO
+static void cf_check evtchn_none_set_pending(
+    struct vcpu *v, struct evtchn *evtchn) {}
+static void cf_check evtchn_none_clear_pending(
+    struct domain *d, struct evtchn *evtchn) {}
+static void cf_check evtchn_none_unmask(
+    struct domain *d, struct evtchn *evtchn) {}
+static bool cf_check evtchn_none_is_pending(
+    const struct domain *d, const struct evtchn *evtchn) { return false; }
+static bool cf_check evtchn_none_is_masked(
+    const struct domain *d, const struct evtchn *evtchn) { return true; }
+static void cf_check evtchn_none_print_state(
+    struct domain *d, const struct evtchn *evtchn) {}
+
+static const struct evtchn_port_ops evtchn_port_ops_none = {
+    .set_pending   = evtchn_none_set_pending,
+    .clear_pending = evtchn_none_clear_pending,
+    .unmask        = evtchn_none_unmask,
+    .is_pending    = evtchn_none_is_pending,
+    .is_masked     = evtchn_none_is_masked,
+    .print_state   = evtchn_none_print_state,
+};
+
+static void evtchn_none_init(struct domain *d)
+{
+    d->evtchn_port_ops = &evtchn_port_ops_none;
+}
+#endif

For arch without 2L supports .is_pending() will return false we can just 
do the following instead of ifdef:
-#ifdef CONFIG_HAS_SHARED_INFO
-        if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
-            evtchn->pending = true;
-#endif
+        if ( evtchn_is_pending(d, evtchn) )
+             evtchn->pending = true;

Would you be okay with this approach instead of ifdef?

~ Oleksii