Message ID | 3ec6be2f15e3815c38a929efaf4efca5f48d4363.1643217384.git.jlanka@nvidia.com |
---|---|
State | New |
Headers | show |
Series | UBUNTU: SAUCE: Fix references to sprintf that may cause buffer overflow | expand |
Adding verification team. -----Original Message----- From: Jitendra Lanka <jlanka@nvidia.com> Sent: Wednesday, January 26, 2022 11:35 AM To: kernel-team@lists.ubuntu.com Cc: Jitendra Lanka <jlanka@nvidia.com> Subject: [SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: Fix references to sprintf that may cause buffer overflow BugLink: https://bugs.launchpad.net/bugs/1959119 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@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) 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)); -- 2.30.1
On 1/26/22 10:34 AM, Jitendra Lanka wrote: > BugLink: https://bugs.launchpad.net/bugs/1959119 > > 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@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. > 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
-----Original Message----- From: Tim Gardner <tim.gardner@canonical.com> Sent: Thursday, January 27, 2022 7:01 AM To: Jitendra Lanka <jlanka@nvidia.com>; kernel-team@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@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
Terminating this thread. v2 is on the way. On 1/27/22 7:39 AM, Jitendra Lanka wrote: > > > -----Original Message----- > From: Tim Gardner <tim.gardner@canonical.com> > Sent: Thursday, January 27, 2022 7:01 AM > To: Jitendra Lanka <jlanka@nvidia.com>; kernel-team@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@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
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) 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));
BugLink: https://bugs.launchpad.net/bugs/1959119 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@nvidia.com> --- drivers/platform/mellanox/mlxbf-pmc.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)