Messages in this thread | | | Date | Wed, 1 Feb 2023 18:44:28 -0600 | Subject | Re: [PATCH] iommu/amd: @Add a length limitation for the ivrs_acpihid command-line parameter | From | Kim Phillips <> |
| |
Not sure what that '@' is doing in the subject line...
On 1/30/23 2:38 AM, Gavrilov Ilia wrote: > The 'acpiid' buffer in the parse_ivrs_acpihid function may overflow, > because the string specifier in the format string sscanf() > has no width limitation. > > Found by InfoTeCS on behalf of Linux Verification Center > (linuxtesting.org) with SVACE. > > Fixes: ca3bf5d47cec ("iommu/amd: Introduces ivrs_acpihid kernel parameter") > Signed-off-by: Ilia.Gavrilov <Ilia.Gavrilov@infotecs.ru>
cc: stable?
> --- > drivers/iommu/amd/init.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > index 467b194975b3..19a46b9f7357 100644 > --- a/drivers/iommu/amd/init.c > +++ b/drivers/iommu/amd/init.c > @@ -3475,15 +3475,26 @@ static int __init parse_ivrs_hpet(char *str) > return 1; > } > > +#define ACPIID_LEN (ACPIHID_UID_LEN + ACPIHID_HID_LEN) > + > static int __init parse_ivrs_acpihid(char *str) > { > u32 seg = 0, bus, dev, fn; > char *hid, *uid, *p, *addr; > - char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0}; > + char acpiid[ACPIID_LEN] = {0}; > int i; > > addr = strchr(str, '@'); > if (!addr) { > + addr = strchr(str, '='); > + if (!addr) > + goto not_found; > + > + ++addr; > + > + if (strlen(addr) > ACPIID_LEN) > + goto not_found; > + > if (sscanf(str, "[%x:%x.%x]=%s", &bus, &dev, &fn, acpiid) == 4 || > sscanf(str, "[%x:%x:%x.%x]=%s", &seg, &bus, &dev, &fn, acpiid) == 5) { > pr_warn("ivrs_acpihid%s option format deprecated; use ivrs_acpihid=%s@%04x:%02x:%02x.%d instead\n", > @@ -3496,6 +3507,9 @@ 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) > + goto not_found; > + > if (sscanf(str, "=%s", acpiid) != 1) > goto not_found; >
That works, or, this fix might be able to be made more brief if we could transform all the sscanf's '%s's to:
"%" __stringify(ACPIID_LEN) "s"
but the latter might make the already long sscanf line lengths longer...
Either way:
Reviewed-by: Kim Phillips <kim.phillips@amd.com>
Kim
| |