git: 4a2494d76ad0 - main - amd_iommu: Bound IVHD device-entry parsing

The branch main has been updated by kbowling:

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

commit 4a2494d76ad03c755f2cb0ed5e1311b03bb2a8e2
Author:     Kevin Bowling <[email protected]>
AuthorDate: 2026-08-06 08:22:37 +0000
Commit:     Kevin Bowling <[email protected]>
CommitDate: 2026-08-23 12:04:09 +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
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D58724
---
 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 d45c576856eb..ba6abae055fa 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);