[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] xen/domain: fix UBSAN null pointer dereference of d->shared_info
On 6/3/26 11:22 AM, Jan Beulich wrote:
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.
For event_2l.c would you be okay to make compilation of it conditional
by CONFIG_HAS_SHARED_INFO or it would be better to introduce separate
CONFIG_HAS_EVTCHN_2L:
-obj-y += event_2l.o
+obj-$(CONFIG_HAS_EVTCHN_2L) += event_2l.o
~ Oleksii
|
