Skip to contentSkip to conversation

iommu/amd: Avoid stack buffer overflow from kernel cmdline

PATCH RESEND In review Kees Cook 3 participants 3 messages Aug 4, 2025 – Aug 15, 2025

Subsystems (2)
IOMMU AMD IOMMU (AMD-Vi)
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH RESEND 1/1 iommu/amd: Avoid stack buffer overflow from kernel cmdline

While the kernel command line is considered trusted in most environments, avoid writing 1 byte past the end of "acpiid" if the "str" argument is maximum length.

Reported-by Simcha Kosman <[email protected]>
Closes https://lore.kernel.org/all/AS8P193MB2271C4B24BCEDA31830F37AE84A52@AS8P193MB2271.EURP193.PROD.OUTLOOK.COM open here
Fixes b6b26d86c61c ("iommu/amd: Add a length limitation for the ivrs_acpihid command-line parameter")
Signed-off-by Kees Cook <[email protected]>

---
This was sent out before but it didn't end up on any public mailing list. My mistake!
Cc: Joerg Roedel <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: <[email protected]>

---
 drivers/iommu/amd/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/amd/init.c +2 -2
index 7b5af6176de9..e11322d8d775 100644
@@ -3638,7 +3638,7 @@ static int __init parse_ivrs_acpihid(char *str)
 {
 	u32 seg = 0, bus, dev, fn;
 	char *hid, *uid, *p, *addr;
-	char acpiid[ACPIID_LEN] = {0};
+	char acpiid[ACPIID_LEN + 1] = { }; /* size with NUL terminator */
 	int i;
 
 	addr = strchr(str, '@');

@@ -3664,7 +3664,7 @@ static int __init parse_ivrs_acpihid(char *str)
 	/* We have the '@', make it the terminator to get just the acpiid */
 	*addr++ = 0;
 
-	if (strlen(str) > ACPIID_LEN + 1)
+	if (strlen(str) > ACPIID_LEN)
 		goto not_found;
 
 	if (sscanf(str, "=%s", acpiid) != 1)
Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#1 lore raw

Hi,

Quoted 24 lines · Kees Cook

While the kernel command line is considered trusted in most environments, avoid writing 1 byte past the end of "acpiid" if the "str" argument is maximum length.

Reported-by Simcha Kosman <[email protected]>
Closes https://lore.kernel.org/all/AS8P193MB2271C4B24BCEDA31830F37AE84A52@AS8P193MB2271.EURP193.PROD.OUTLOOK.COM open here
Fixes b6b26d86c61c ("iommu/amd: Add a length limitation for the ivrs_acpihid command-line parameter")
Signed-off-by Kees Cook <[email protected]>

---
This was sent out before but it didn't end up on any public mailing list. My mistake!
Cc: Joerg Roedel <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: <[email protected]>

---
 drivers/iommu/amd/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/amd/init.c +0 -0
index 7b5af6176de9..e11322d8d775 100644

drivers/iommu/amd/init.c static int __init parse_ivrs_acpihid(char *str) · line 3641

Quoted 5 lines
drivers/iommu/amd/init.c +1 -1
@@ -3638,7 +3638,7 @@ static int __init parse_ivrs_acpihid(char *str)
 {
 	u32 seg = 0, bus, dev, fn;
 	char *hid, *uid, *p, *addr;
-	char acpiid[ACPIID_LEN] = {0};
+	char acpiid[ACPIID_LEN + 1] = { }; /* size with NUL terminator */

minor nits below,

s/NUL/NULL
and keeping "{0}" will be as per standard.

otherwise looks good!

Reviewed-by Ankit Soni <[email protected]>
Quoted 15 lines
drivers/iommu/amd/init.c +1 -1
 	int i;
 
 	addr = strchr(str, '@');

@@ -3664,7 +3664,7 @@ static int __init parse_ivrs_acpihid(char *str)
 	/* We have the '@', make it the terminator to get just the acpiid */
 	*addr++ = 0;
 
-	if (strlen(str) > ACPIID_LEN + 1)
+	if (strlen(str) > ACPIID_LEN)
 		goto not_found;
 
 	if (sscanf(str, "=%s", acpiid) != 1)
Signature · 1 line
2.34.1
maintainer
#2 lore raw
In reply to Kees Cook
Quoted 2 lines · Kees Cook
 drivers/iommu/amd/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied for -rc, thanks Kees.