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 2:33 PM, Jan Beulich wrote:
> On 03.06.2026 13:57, Oleksii Kurochko wrote:
> > On 6/3/26 1:23 PM, Jan Beulich wrote:
> > > On 03.06.2026 13:05, Oleksii Kurochko wrote:
> > > > 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.
> > > I don't understand this part.
> > If the change will look like:
> >
> >     if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) && guest_test_bit(d, port,
> > &shared_info(d, evtchn_pending)) )
> >         evtchn->pending = true;
> >
> > It will help to avoid NULL pointer dereference of shared info page in
> > case of 2L isn't supported. But considering that shared_info() macros
> > will be reworked in the way which will lead to linkage error in the case
> > when it is used and arch doesn't have shared info page support usage of
> > shared_info() in setup_ports() will lead to linkage error what is wanted
> > to be avoid for arch without 2L support.
> What linking error are you talking of? When !HAS_SHARED_INFO, the compiler
> will DCE the entire guest_test_bit(), including the shared_info() use. IOW
> I still don't understand what you're talking of.
Oh, you are right. Sorry for the noise.

Thanks.

~ Oleksii