git: 792307d8c31a - stable/15 - amd_iommu: Bound IVHD device-entry parsing

From: Kevin Bowling <kbowling_at_FreeBSD.org>
Date: Sun, 06 Sep 2026 01:20:57 UTC
The branch stable/15 has been updated by kbowling:

URL: https://cgit.FreeBSD.org/src/commit/?id=792307d8c31aae4e1fe2be945561d64f0d38e6bb

commit 792307d8c31aae4e1fe2be945561d64f0d38e6bb
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-06 08:22:37 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-06 01:20:42 +0000

    amd_iommu: Bound IVHD device-entry parsing
    
    Validate the IVRS table and every subtable length before using either
    to form iterator bounds. Reject truncated typed IVHD blocks instead of
    passing them to a type-specific callback.
    
    Within each IVHD payload, correct the lower-bound comparison for
    extended range entries and validate fixed-size entries, paired range
    terminators, the fixed HID body, and the variable HID UID before
    dereferencing or advancing. Malformed firmware can no longer drive
    either iterator beyond its enclosing object.
    
    Reviewed by:    kib
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D58724
    
    (cherry picked from commit 4a2494d76ad03c755f2cb0ed5e1311b03bb2a8e2)
---
 sys/x86/iommu/amd_drv.c | 132 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 99 insertions(+), 33 deletions(-)

diff --git a/sys/x86/iommu/amd_drv.c b/sys/x86/iommu/amd_drv.c
index f2a9bc518201..3a8c1cab7353 100644
--- a/sys/x86/iommu/amd_drv.c
+++ b/sys/x86/iommu/amd_drv.c
@@ -91,9 +91,14 @@ amdiommu_ivrs_iterate_tbl_typed(amdiommu_itercc_t iter, void *arg,
     int type, ACPI_TABLE_IVRS *ivrs_tbl)
 {
 	char *ptr, *ptrend;
+	size_t min_length, remaining;
 	bool done;
 
 	done = false;
+	if (ivrs_tbl->Header.Length < sizeof(*ivrs_tbl)) {
+		printf("amdiommu_iterate_tbl: truncated IVRS table\n");
+		return (done);
+	}
 	ptr = (char *)ivrs_tbl + sizeof(*ivrs_tbl);
 	ptrend = (char *)ivrs_tbl + ivrs_tbl->Header.Length;
 	for (;;) {
@@ -101,18 +106,42 @@ amdiommu_ivrs_iterate_tbl_typed(amdiommu_itercc_t iter, void *arg,
 
 		if (ptr >= ptrend)
 			break;
+		remaining = ptrend - ptr;
+		if (remaining < sizeof(*ivrsh)) {
+			printf("amdiommu_iterate_tbl: truncated IVRS subtable header\n");
+			break;
+		}
 		ivrsh = (ACPI_IVRS_HEADER *)ptr;
-		if (ivrsh->Length <= 0) {
+		if (ivrsh->Length < sizeof(*ivrsh) ||
+		    ivrsh->Length > remaining) {
 			printf("amdiommu_iterate_tbl: corrupted IVRS table, "
-			    "length %d\n", ivrsh->Length);
+			    "subtable length %u, remaining %zu\n",
+			    ivrsh->Length, remaining);
 			break;
 		}
-		ptr += ivrsh->Length;
-		if (ivrsh->Type ==  type) {
+		switch (ivrsh->Type) {
+		case ACPI_IVRS_TYPE_HARDWARE2:
+		case ACPI_IVRS_TYPE_HARDWARE3:
+			min_length = sizeof(ACPI_IVRS_HARDWARE2);
+			break;
+		case ACPI_IVRS_TYPE_HARDWARE1:
+			min_length = sizeof(ACPI_IVRS_HARDWARE1);
+			break;
+		default:
+			min_length = sizeof(ACPI_IVRS_HEADER);
+			break;
+		}
+		if (ivrsh->Length < min_length) {
+			printf("amdiommu_iterate_tbl: truncated IVRS "
+			    "subtable type %#x\n", ivrsh->Type);
+			break;
+		}
+		if (ivrsh->Type == type) {
 			done = iter((void *)ivrsh, arg);
 			if (done)
 				break;
 		}
+		ptr += ivrsh->Length;
 	}
 	return (done);
 }
@@ -657,32 +686,43 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
     struct ivhd_find_unit *ifu)
 {
 	char *db, *de;
+	size_t remaining;
 	size_t len;
 
-	for (de = (char *)d + tlen; (char *)d < de;
-	     d = (ACPI_IVRS_DE_HEADER *)(db + len)) {
-		db = (char *)d;
+	db = (char *)d;
+	de = db + tlen;
+	while (db < de) {
+		remaining = de - db;
+		d = (ACPI_IVRS_DE_HEADER *)db;
+		if (remaining < sizeof(*d)) {
+			printf("amdiommu: truncated IVRS device entry header\n");
+			return (false);
+		}
+
 		if (d->Type == ACPI_IVRS_TYPE_PAD4) {
 			len = sizeof(ACPI_IVRS_DEVICE4);
 		} else if (d->Type == ACPI_IVRS_TYPE_ALL) {
 			ACPI_IVRS_DEVICE4 *d4;
 
-			d4 = (ACPI_IVRS_DEVICE4 *)db;
 			len = sizeof(*d4);
+			d4 = (ACPI_IVRS_DEVICE4 *)db;
 			ifu->dte = d4->Header.DataSetting;
 		} else if (d->Type == ACPI_IVRS_TYPE_SELECT) {
 			ACPI_IVRS_DEVICE4 *d4;
 
+			len = sizeof(*d4);
 			d4 = (ACPI_IVRS_DEVICE4 *)db;
 			if (d4->Header.Id == ifu->rid) {
 				ifu->dte = d4->Header.DataSetting;
 				ifu->rid_real = ifu->rid;
 				return (true);
 			}
-			len = sizeof(*d4);
 		} else if (d->Type == ACPI_IVRS_TYPE_START) {
 			ACPI_IVRS_DEVICE4 *d4, *d4n;
 
+			len = 2 * sizeof(*d4);
+			if (len > remaining)
+				goto truncated;
 			d4 = (ACPI_IVRS_DEVICE4 *)db;
 			d4n = d4 + 1;
 			if (d4n->Header.Type != ACPI_IVRS_TYPE_END) {
@@ -696,23 +736,29 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
 				ifu->rid_real = ifu->rid;
 				return (true);
 			}
-			len = 2 * sizeof(*d4);
 		} else if (d->Type == ACPI_IVRS_TYPE_PAD8) {
 			len = sizeof(ACPI_IVRS_DEVICE8A);
+			if (len > remaining)
+				goto truncated;
 		} else if (d->Type == ACPI_IVRS_TYPE_ALIAS_SELECT) {
 			ACPI_IVRS_DEVICE8A *d8a;
 
+			len = sizeof(*d8a);
+			if (len > remaining)
+				goto truncated;
 			d8a = (ACPI_IVRS_DEVICE8A *)db;
 			if (d8a->Header.Id == ifu->rid) {
 				ifu->dte = d8a->Header.DataSetting;
 				ifu->rid_real = d8a->UsedId;
 				return (true);
 			}
-			len = sizeof(*d8a);
 		} else if (d->Type == ACPI_IVRS_TYPE_ALIAS_START) {
 			ACPI_IVRS_DEVICE8A *d8a;
 			ACPI_IVRS_DEVICE4 *d4;
 
+			len = sizeof(*d8a) + sizeof(*d4);
+			if (len > remaining)
+				goto truncated;
 			d8a = (ACPI_IVRS_DEVICE8A *)db;
 			d4 = (ACPI_IVRS_DEVICE4 *)(d8a + 1);
 			if (d4->Header.Type != ACPI_IVRS_TYPE_END) {
@@ -726,10 +772,12 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
 				ifu->rid_real = d8a->UsedId;
 				return (true);
 			}
-			len = sizeof(*d8a) + sizeof(*d4);
 		} else if (d->Type == ACPI_IVRS_TYPE_EXT_SELECT) {
 			ACPI_IVRS_DEVICE8B *d8b;
 
+			len = sizeof(*d8b);
+			if (len > remaining)
+				goto truncated;
 			d8b = (ACPI_IVRS_DEVICE8B *)db;
 			if (d8b->Header.Id == ifu->rid) {
 				ifu->dte = d8b->Header.DataSetting;
@@ -737,11 +785,13 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
 				ifu->edte = d8b->ExtendedData;
 				return (true);
 			}
-			len = sizeof(*d8b);
 		} else if (d->Type == ACPI_IVRS_TYPE_EXT_START) {
 			ACPI_IVRS_DEVICE8B *d8b;
 			ACPI_IVRS_DEVICE4 *d4;
 
+			len = sizeof(*d8b) + sizeof(*d4);
+			if (len > remaining)
+				goto truncated;
 			d8b = (ACPI_IVRS_DEVICE8B *)db;
 			d4 = (ACPI_IVRS_DEVICE4 *)(db + sizeof(*d8b));
 			if (d4->Header.Type != ACPI_IVRS_TYPE_END) {
@@ -749,17 +799,19 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
 				    "(%#x)\n", d4->Header.Type);
 				return (false);
 			}
-			if (d8b->Header.Id >= ifu->rid &&
+			if (d8b->Header.Id <= ifu->rid &&
 			    ifu->rid <= d4->Header.Id) {
 				ifu->dte = d8b->Header.DataSetting;
 				ifu->rid_real = ifu->rid;
 				ifu->edte = d8b->ExtendedData;
 				return (true);
 			}
-			len = sizeof(*d8b) + sizeof(*d4);
 		} else if (d->Type == ACPI_IVRS_TYPE_SPECIAL) {
 			ACPI_IVRS_DEVICE8C *d8c;
 
+			len = sizeof(*d8c);
+			if (len > remaining)
+				goto truncated;
 			d8c = (ACPI_IVRS_DEVICE8C *)db;
 			if (((ifu->type == IFU_DEV_IOAPIC &&
 			    d8c->Variety == ACPI_IVHD_IOAPIC) ||
@@ -770,31 +822,39 @@ amdiommu_find_unit_scan_ivrs(ACPI_IVRS_DE_HEADER *d, size_t tlen,
 				ifu->rid_real = d8c->UsedId;
 				return (true);
 			}
-			len = sizeof(*d8c);
 		} else if (d->Type == ACPI_IVRS_TYPE_HID) {
 			ACPI_IVRS_DEVICE_HID *dh;
 
+			len = sizeof(*dh);
+			if (len > remaining) {
+				printf("amdiommu: truncated IVRS HID entry\n");
+				return (false);
+			}
 			dh = (ACPI_IVRS_DEVICE_HID *)db;
-			len = sizeof(*dh) + dh->UidLength;
-			/* XXXKIB */
-		} else {
-#if 0
-			printf("amdiommu: unknown IVRS device entry type %#x\n",
-			    d->Type);
-#endif
-			if (d->Type <= 63)
-				len = sizeof(ACPI_IVRS_DEVICE4);
-			else if (d->Type <= 127)
-				len = sizeof(ACPI_IVRS_DEVICE8A);
-			else {
-				printf("amdiommu: abort, cannot "
-				    "advance iterator, item type %#x\n",
-				    d->Type);
+			if (dh->UidLength > remaining - len) {
+				printf("amdiommu: truncated IVRS HID UID\n");
 				return (false);
 			}
+			len += dh->UidLength;
+			/* XXXKIB */
+		} else if (d->Type <= 63) {
+			len = sizeof(ACPI_IVRS_DEVICE4);
+		} else if (d->Type <= 127) {
+			len = sizeof(ACPI_IVRS_DEVICE8A);
+			if (len > remaining)
+				goto truncated;
+		} else {
+			printf("amdiommu: abort, cannot advance iterator, "
+			    "item type %#x\n", d->Type);
+			return (false);
 		}
+		db += len;
 	}
 	return (false);
+
+truncated:
+	printf("amdiommu: truncated IVRS device entry %#x\n", d->Type);
+	return (false);
 }
 
 static bool
@@ -810,8 +870,11 @@ amdiommu_find_unit_scan_0x11(ACPI_IVRS_HARDWARE2 *ivrs, void *arg)
 
 	if (ifu->domain != ivrs->PciSegmentGroup)
 		return (false);
+	if (ivrs->Header.Length < sizeof(*ivrs))
+		return (false);
 	d = (ACPI_IVRS_DE_HEADER *)(ivrs + 1);
-	res = amdiommu_find_unit_scan_ivrs(d, ivrs->Header.Length, ifu);
+	res = amdiommu_find_unit_scan_ivrs(d,
+	    ivrs->Header.Length - sizeof(*ivrs), ifu);
 	if (res)
 		ifu->device_id = ivrs->Header.DeviceId;
 	return (res);
@@ -829,8 +892,11 @@ amdiommu_find_unit_scan_0x10(ACPI_IVRS_HARDWARE1 *ivrs, void *arg)
 
 	if (ifu->domain != ivrs->PciSegmentGroup)
 		return (false);
+	if (ivrs->Header.Length < sizeof(*ivrs))
+		return (false);
 	d = (ACPI_IVRS_DE_HEADER *)(ivrs + 1);
-	res = amdiommu_find_unit_scan_ivrs(d, ivrs->Header.Length, ifu);
+	res = amdiommu_find_unit_scan_ivrs(d,
+	    ivrs->Header.Length - sizeof(*ivrs), ifu);
 	if (res)
 		ifu->device_id = ivrs->Header.DeviceId;
 	return (res);