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

[Tim Gardner]

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

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