On 09/10/2018 04:30 AM, Daniel Thompson wrote:
> On Fri, Sep 07, 2018 at 06:08:56AM -0400, Prarit Bhargava wrote:
>> gcc 8.1.0 warns with:
>>
>> kernel/debug/kdb/kdb_support.c: In function ‘kallsyms_symbol_next’:
>> kernel/debug/kdb/kdb_support.c:239:4: warning: ‘strncpy’ specified bound
>> depends on the length of the source argument [-Wstringop-overflow=]
>> strncpy(prefix_name, name, strlen(name)+1);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/debug/kdb/kdb_support.c:239:31: note: length computed here
>>
>> The strings do not need to be zero padded so use strlcpy() instead.
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> Cc: Jonathan Toppins <jtopp...@redhat.com>
>> Cc: Jason Wessel <jason.wes...@windriver.com>
>> Cc: Daniel Thompson <daniel.thomp...@linaro.org>
>> ---
>> kernel/debug/kdb/kdb_support.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
>> index 990b3cc526c8..1ad4370ccbf0 100644
>> --- a/kernel/debug/kdb/kdb_support.c
>> +++ b/kernel/debug/kdb/kdb_support.c
>> @@ -236,7 +236,7 @@ int kallsyms_symbol_next(char *prefix_name, int flag)
>>
>> while ((name = kdb_walk_kallsyms(&pos))) {
>> if (strncmp(name, prefix_name, prefix_len) == 0) {
>> - strncpy(prefix_name, name, strlen(name)+1);
>> + strlcpy(prefix_name, name, strlen(name)+1);
>
> How does this *fix* the warning?
>
> The warning occurs because a "safe" string copy function is incorrectly
> using the length of the second argument as the length (i.e. it is simply
> an inefficient implementation of strcpy). The code is still bogus
> whether you use strncpy, strlcpy or strscpy. All we are doing here is
> kicking the ball down the road until someone teaches gcc 9+ about
> strlcpy()!
>
Hi Daniel, you are correct in that, however name is always smaller in size than
prefix_name[1]. I assumed (badly) that the code had taken the sizes into
account when written but wasn't sure, hence the simple strlcpy() patch.
In some other cases in the kernel it was okay to make this type of change but
clearly not here (see the gcc 8.1.0 patches in the kernel). Another option is
to drop strcpy and use memcpy, as Guenter has done with some other fixes. TBH I
thought that was the correct fix but made a bad assumption about the history of
the code. What do you think of the memcpy replacement?
P.
>
> Daniel.
>
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
