Skip to content

Commit e461b8c

Browse files
amonakovjoergroedel
authored andcommitted
iommu/amd: Fix over-read of ACPI UID from IVRS table
IVRS parsing code always tries to read 255 bytes from memory when retrieving ACPI device path, and makes an assumption that firmware provides a zero-terminated string. Both of those are bugs: the entry is likely to be shorter than 255 bytes, and zero-termination is not guaranteed. With Acer SF314-42 firmware these issues manifest visibly in dmesg: AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR0\xf0\xa5, rdevid:160 AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR1\xf0\xa5, rdevid:160 AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR2\xf0\xa5, rdevid:160 AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR3>\x83e\x8d\x9a\xd1... The first three lines show how the code over-reads adjacent table entries into the UID, and in the last line it even reads garbage data beyond the end of the IVRS table itself. Since each entry has the length of the UID (uidl member of ivhd_entry struct), use that for memcpy, and manually add a zero terminator. Avoid zero-filling hid and uid arrays up front, and instead ensure the uid array is always zero-terminated. No change needed for the hid array, as it was already properly zero-terminated. Fixes: 2a0cb4e ("iommu/amd: Add new map for storing IVHD dev entry type HID") Signed-off-by: Alexander Monakov <[email protected]> Cc: Joerg Roedel <[email protected]> Cc: [email protected] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent 2ef96a5 commit e461b8c

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

drivers/iommu/amd_iommu_init.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,8 +1329,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
13291329
}
13301330
case IVHD_DEV_ACPI_HID: {
13311331
u16 devid;
1332-
u8 hid[ACPIHID_HID_LEN] = {0};
1333-
u8 uid[ACPIHID_UID_LEN] = {0};
1332+
u8 hid[ACPIHID_HID_LEN];
1333+
u8 uid[ACPIHID_UID_LEN];
13341334
int ret;
13351335

13361336
if (h->type != 0x40) {
@@ -1347,6 +1347,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
13471347
break;
13481348
}
13491349

1350+
uid[0] = '\0';
13501351
switch (e->uidf) {
13511352
case UID_NOT_PRESENT:
13521353

@@ -1361,8 +1362,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
13611362
break;
13621363
case UID_IS_CHARACTER:
13631364

1364-
memcpy(uid, (u8 *)(&e->uid), ACPIHID_UID_LEN - 1);
1365-
uid[ACPIHID_UID_LEN - 1] = '\0';
1365+
memcpy(uid, &e->uid, e->uidl);
1366+
uid[e->uidl] = '\0';
13661367

13671368
break;
13681369
default:

0 commit comments

Comments
 (0)