lkml.org 
[lkml]   [2023]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] iommu/amd: @Add a length limitation for the ivrs_acpihid command-line parameter
From
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

\
 
 \ /
  Last update: 2023-03-27 00:06    [W:0.063 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site