[SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: Fix references to sprintf that may cause buffer overflow

[Jitendra Lanka]

-----Original Message-----
From: Tim Gardner <tim.gardner at canonical.com> 
Sent: Thursday, January 27, 2022 7:01 AM
To: Jitendra Lanka <jlanka at nvidia.com>; kernel-team at lists.ubuntu.com
Subject: Re: [SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: Fix references to sprintf that may cause buffer overflow

External email: Use caution opening links or attachments


On 1/26/22 10:34 AM, Jitendra Lanka wrote:
> BugLink: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .launchpad.net%2Fbugs%2F1959119&data=04%7C01%7Cjlanka%40nvidia.com
> %7Caa94ca2bdc4d4a3d9e8408d9e1950ebd%7C43083d15727340c1b7db39efd9ccc17a
> %7C0%7C0%7C637788853452105926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Rp
> jXrD%2FHEcDyU8cQHxctZn6eesEU2WI4RaDqbEw9s88%3D&reserved=0
>
> Replace sprintf with snprintf containing a defined boundary of 
> PAGE_SIZE for sysfs store/show functions and max array size defined 
> otherwise.
>
> Signed-off-by: Jitendra Lanka <jlanka at nvidia.com>
>
> ---
>   drivers/platform/mellanox/mlxbf-pmc.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c 
> b/drivers/platform/mellanox/mlxbf-pmc.c
> index 493d72a53ee4..eb878b89169a 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -681,7 +681,7 @@ static ssize_t mlxbf_counter_read(struct kobject *ko,
>       } else
>               return -EINVAL;
>
> -     return sprintf(buf, "0x%llx\n", value);
> +     return snprintf(buf, PAGE_SIZE, "0x%llx\n", value);
>   }
>
>   /* Store function for "counter" sysfs files */ @@ -758,7 +758,7 @@ 
> static ssize_t mlxbf_event_find(struct kobject *ko,
>
>       evt_name = mlxbf_pmc_get_event_name((char *)ko->name, evt_num);
>
> -     return sprintf(buf, "0x%llx: %s\n", evt_num, evt_name);
> +     return snprintf(buf, PAGE_SIZE, "0x%llx: %s\n", evt_num, 
> + evt_name);
>   }
>
>   /* Store function for "event" sysfs files */ @@ -811,8 +811,11 @@ 
> static ssize_t mlxbf_print_event_list(struct kobject *ko,
>
>       buf[0] = '\0';
>       while (events[i].evt_name != NULL) {
> -             size += sprintf(e_info, "%x: %s\n", events[i].evt_num,
> -                     events[i].evt_name);
> +             size += snprintf(e_info,
> +                              sizeof(e_info),
> +                              "%x: %s\n",
> +                              events[i].evt_num,
> +                              events[i].evt_name);
>               if (size > PAGE_SIZE)

> Shouldn't this be 'if (size >= PAGE_SIZE)' ? If there is no room for '\0', then its an unterminated string. snprintf() will not write a '\0'
> if the output was truncated.

Thanks Tim. Will post a new patch to address this.

>                       break;
>               strcat(buf, e_info);
> @@ -840,7 +843,7 @@ static ssize_t mlxbf_show_counter_state(struct 
> kobject *ko,
>
>       value = FIELD_GET(MLXBF_L3C_PERF_CNT_CFG__EN, perfcnt_cfg);
>
> -     return sprintf(buf, "%d\n", value);
> +     return snprintf(buf, PAGE_SIZE, "%d\n", value);
>   }
>
>   /* Store function for "enable" sysfs files - only for l3cache */ @@ 
> -1250,4 +1253,4 @@ module_platform_driver(pmc_driver);
>   MODULE_AUTHOR("Mellanox Technologies");
>   MODULE_DESCRIPTION("Mellanox PMC driver");
>   MODULE_LICENSE("Dual BSD/GPL");
> -MODULE_VERSION(__stringify(DRIVER_VERSION));
> \ No newline at end of file
> +MODULE_VERSION(__stringify(DRIVER_VERSION));

rtg
 --
-----------
Tim Gardner
Canonical, Inc