⬢ DragonFlyBSD Kernel Audit
DF-5502 / fix.diff
← back to finding ↓ download raw
--- a/sys/platform/pc64/apic/ioapic.c
+++ b/sys/platform/pc64/apic/ioapic.c
@@ -231,12 +231,34 @@
 	struct ioapic_info *info, *ninfo;
 	int gsi_end;
 
+	/*
+	 * Validate the firmware/driver-supplied topology before using it.
+	 *
+	 * gsi_base and npin arrive raw from ACPI MADT IO_APIC entries
+	 * (acpi_madt.c) and from MPTable entries (mptable.c) and were never
+	 * validated: GlobalIrqBase is a UINT32 truncated to int, so bases
+	 * >= 0x80000000 become negative GSI bases, and "gsi_base + npin - 1"
+	 * overflows for large bases.  A negative GSI reaching
+	 * ioapic_gsi_setup() OOB-indexes ioapic_conf.ioc_intsrc[] and
+	 * forwards negative IRQs to ioapic_set_legacy_irqmap(), which
+	 * writes ioapic_irqmaps[] and ioapic_irqs[] out of bounds on
+	 * production (no-INVARIANTS) builds.
+	 */
+	if (gsi_base < 0 || npin <= 0 || gsi_base > 0x7fffffff - npin) {
+		kprintf("ioapic_add: invalid I/O APIC gsi base %d, npin %d, "
+			"ignored\n", gsi_base, npin);
+		return;
+	}
+
 	gsi_end = gsi_base + npin - 1;
 	TAILQ_FOREACH(info, &ioapic_conf.ioc_list, io_link) {
-		if ((gsi_base >= info->io_gsi_base &&
-		     gsi_base < info->io_gsi_base + info->io_npin) ||
-		    (gsi_end >= info->io_gsi_base &&
-		     gsi_end < info->io_gsi_base + info->io_npin)) {
+		/*
+		 * Full interval-overlap test; the old two-sided membership
+		 * test missed the containment case (a new range fully
+		 * swallowing an existing one).
+		 */
+		if (gsi_base < info->io_gsi_base + info->io_npin &&
+		    gsi_end >= info->io_gsi_base) {
 			panic("ioapic_add: overlapped gsi, base %d npin %d, "
 			      "hit base %d, npin %d\n", gsi_base, npin,
 			      info->io_gsi_base, info->io_npin);
@@ -348,6 +370,22 @@
 		return;
 	}
 
+	/*
+	 * Defensive: a negative GSI must never take the intsrc scan (it
+	 * aliases the ioc_intsrc[].int_gsi == -1 sentinel) or the default
+	 * identity mapping (negative ioc_intsrc[] index, and a negative
+	 * IRQ forwarded to ioapic_set_legacy_irqmap()).  ioapic_add()
+	 * already rejects negative GSI bases; keep the pin masked and stop
+	 * here if one ever gets through.
+	 */
+	if (gsi < 0) {
+		imen_lock();
+		ioapic_pin_setup(ioapic_gsi_ioaddr(gsi), ioapic_gsi_pin(gsi),
+		    0, INTR_TRIGGER_EDGE, INTR_POLARITY_HIGH, 0);
+		imen_unlock();
+		return;
+	}
+
 	trig = 0;	/* silence older gcc's */
 	pola = 0;	/* silence older gcc's */
 
@@ -368,7 +406,7 @@
 		 * use the default 1:1 mapping
 		 */
 		irq = gsi;
-		if (irq < ISA_IRQ_CNT) {
+		if (irq >= 0 && irq < ISA_IRQ_CNT) {
 			if (ioapic_conf.ioc_intsrc[irq].int_gsi >= 0) {
 				/*
 				 * This IRQ is mapped to different GSI,