|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v2 2/3] xen/domain: fix UBSAN null pointer dereference of d->shared_info
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.
- Update the commit message.
- Protect some other places in common code from NULL pointer deref of
d->shared_info.
- Drop R-by: Baptiste ... as some extra checks are added.
---
xen/common/domain.c | 2 +-
xen/common/event_fifo.c | 3 ++-
xen/common/time.c | 3 +++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index bb9e210c2895..e64b7df9b704 100644
--- 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);
}
diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 37cba9bc4564..59d9bf4c7ec0 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -562,7 +562,8 @@ static void setup_ports(struct domain *d, unsigned int
prev_evtchns)
evtchn = evtchn_from_port(d, port);
- if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
+ if ( d->shared_info &&
+ guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
evtchn->pending = true;
evtchn_fifo_set_priority(d, evtchn, EVTCHN_FIFO_PRIORITY_DEFAULT);
diff --git a/xen/common/time.c b/xen/common/time.c
index 04a65f00b35c..1ee49a8b0d13 100644
--- a/xen/common/time.c
+++ b/xen/common/time.c
@@ -94,6 +94,9 @@ void update_domain_wallclock_time(struct domain *d)
uint32_t *wc_version;
uint64_t sec;
+ if ( !d->shared_info )
+ return;
+
spin_lock(&wc_lock);
wc_version = &shared_info(d, wc_version);
--
2.54.0
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |