git: 4ece79968e70 - main - x86/xen: fix out of bounds access to the event channel masks on resume

The branch main has been updated by royger:

URL: https://cgit.FreeBSD.org/src/commit/?id=4ece79968e70b96d3283f202635d49a4bf04a7e7

commit 4ece79968e70b96d3283f202635d49a4bf04a7e7
Author:     Roger Pau Monné <royger@FreeBSD.org>
AuthorDate: 2024-02-05 10:47:25 +0000
Commit:     Roger Pau Monné <royger@FreeBSD.org>
CommitDate: 2024-02-22 10:08:03 +0000

    x86/xen: fix out of bounds access to the event channel masks on resume
    
    When resuming from migration or suspension all regular event channels ports are
    reset to the INVALID_EVTCHN value, and drivers should re-initialize them
    according to the new value provided by the other end of the connection.
    
    However, the driver would first attempt to unbind the event channel handler
    before attempting to bind it using the newly provided port.  This unbind uses
    the stale event channel port that has been set to INVALID_EVTCHN for some
    operations (notably as a result of the handler removal the interrupt subsystem
    ends up calling disable intr and source PIC hooks).
    
    This was fine when INVALID_EVTCHN was 0, as then the operation would just
    result in pointless setting of the 0 bit in the different event channel related
    control arrays (evtchn_{pending,mask} for example).  However with the change to
    define INVALID_EVTCHN as ~0 the write is no longer pointless, and we end up
    triggering a page-fault, or corrupting random data that happens to be mapped at
    the array position + ~0 bits.
    
    In hindsight the change of INVALID_EVTCHN from 0 to ~0 was way more risky than
    initially assessed, and I believe has end up resulting in more fragile code for
    no real benefit.
    
    Fix the disable intr and source wrappers to check whether the event channel is
    valid before attempting to use it.
    
    Also introduce some extra KASSERTs in several array accesses in order to avoid
    out of bounds accesses if INVALID_EVTCHN ever reaches those functions.
    
    Fixes: 1797ff962769 ('xen/intr: cleanup event channel number use')
    MFC after: 1 week
    Sponsored by: Cloud Software Group
    Reviewed by: markj
    Differential revision: https://reviews.freebsd.org/D43928
---
 sys/dev/xen/bus/xen_intr.c | 8 ++++++--
 sys/xen/evtchn/evtchnvar.h | 7 +++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/sys/dev/xen/bus/xen_intr.c b/sys/dev/xen/bus/xen_intr.c
index 3fc8fb0fe83a..609f31b5418a 100644
--- a/sys/dev/xen/bus/xen_intr.c
+++ b/sys/dev/xen/bus/xen_intr.c
@@ -166,6 +166,7 @@ evtchn_cpu_mask_port(u_int cpu, evtchn_port_t port)
 	struct xen_intr_pcpu_data *pcpu;
 
 	pcpu = DPCPU_ID_PTR(cpu, xen_intr_pcpu);
+	KASSERT(is_valid_evtchn(port), ("Invalid event channel port"));
 	xen_clear_bit(port, pcpu->evtchn_enabled);
 }
 
@@ -188,6 +189,7 @@ evtchn_cpu_unmask_port(u_int cpu, evtchn_port_t port)
 	struct xen_intr_pcpu_data *pcpu;
 
 	pcpu = DPCPU_ID_PTR(cpu, xen_intr_pcpu);
+	KASSERT(is_valid_evtchn(port), ("Invalid event channel port"));
 	xen_set_bit(port, pcpu->evtchn_enabled);
 }
 
@@ -619,7 +621,8 @@ void
 xen_intr_disable_intr(struct xenisrc *isrc)
 {
 
-	evtchn_mask_port(isrc->xi_port);
+	if (__predict_true(is_valid_evtchn(isrc->xi_port)))
+		evtchn_mask_port(isrc->xi_port);
 }
 
 /**
@@ -706,7 +709,8 @@ xen_intr_disable_source(struct xenisrc *isrc)
 	 * unmasked by the generic interrupt code. The event channel
 	 * device will unmask them when needed.
 	 */
-	isrc->xi_masked = !!evtchn_test_and_set_mask(isrc->xi_port);
+	if (__predict_true(is_valid_evtchn(isrc->xi_port)))
+		isrc->xi_masked = !!evtchn_test_and_set_mask(isrc->xi_port);
 }
 
 /*
diff --git a/sys/xen/evtchn/evtchnvar.h b/sys/xen/evtchn/evtchnvar.h
index 455f7bcbd620..3e6575748f6b 100644
--- a/sys/xen/evtchn/evtchnvar.h
+++ b/sys/xen/evtchn/evtchnvar.h
@@ -37,8 +37,11 @@
 #include <contrib/xen/event_channel.h>
 
 /* Macros for accessing event channel values */
-#define	EVTCHN_PTR(type, port) \
-	(HYPERVISOR_shared_info->evtchn_##type + ((port) / __LONG_BIT))
+#define	EVTCHN_PTR(type, port) ({ 					\
+	KASSERT(port < nitems(HYPERVISOR_shared_info->evtchn_##type) *	\
+	    sizeof(xen_ulong_t) * 8, ("Invalid event channel port"));	\
+	(HYPERVISOR_shared_info->evtchn_##type + ((port) / __LONG_BIT));\
+})
 #define	EVTCHN_BIT(port)	((port) & (__LONG_BIT - 1))
 #define	EVTCHN_MASK(port)	(1UL << EVTCHN_BIT(port))