diff mbox series

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

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

Commit Message

Jitendra Lanka Jan. 26, 2022, 5:34 p.m. UTC 
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(-)

Comments

Jitendra Lanka Jan. 26, 2022, 5:43 p.m. UTC | #1 
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
Tim Gardner Jan. 27, 2022, 1 p.m. UTC | #2 
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
Jitendra Lanka Jan. 27, 2022, 2:39 p.m. UTC | #3 
-----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&amp;data=04%7C01%7Cjlanka%40nvidia.com
> %7Caa94ca2bdc4d4a3d9e8408d9e1950ebd%7C43083d15727340c1b7db39efd9ccc17a
> %7C0%7C0%7C637788853452105926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Rp
> jXrD%2FHEcDyU8cQHxctZn6eesEU2WI4RaDqbEw9s88%3D&amp;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
Tim Gardner Jan. 27, 2022, 3:39 p.m. UTC | #4 
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&amp;data=04%7C01%7Cjlanka%40nvidia.com
>> %7Caa94ca2bdc4d4a3d9e8408d9e1950ebd%7C43083d15727340c1b7db39efd9ccc17a
>> %7C0%7C0%7C637788853452105926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Rp
>> jXrD%2FHEcDyU8cQHxctZn6eesEU2WI4RaDqbEw9s88%3D&amp;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 mbox series

Patch

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));