Re: [PATCH v2] kdb: Use strscpy with destination buffer size

[Prarit Bhargava]

On 10/02/2018 11:53 AM, Daniel Thompson wrote:
> On 20/09/2018 13:59, 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
>>
>> Use strscpy() with the destination buffer size, and use ellipses when
>> displaying truncated symbols.
>>
>> v2: Use strscpy()
>>
>> Signed-off-by: Prarit Bhargava <prarit@xxxxxxxxxx>
>> Cc: Jonathan Toppins <jtoppins@xxxxxxxxxx>
>> Cc: Jason Wessel <jason.wessel@xxxxxxxxxxxxx>
>> Cc: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
>> Cc: kgdb-bugreport@xxxxxxxxxxxxxxxxxxxxx
>> ---
>> Â kernel/debug/kdb/kdb_io.cÂÂÂÂÂ | 15 +++++++++------
>> Â kernel/debug/kdb/kdb_private.h |Â 2 +-
>> Â kernel/debug/kdb/kdb_support.c | 10 +++++-----
>> Â 3 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
>> index ed5d34925ad0..6a4b41484afe 100644
>> --- a/kernel/debug/kdb/kdb_io.c
>> +++ b/kernel/debug/kdb/kdb_io.c
>> @@ -216,7 +216,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
>> ÂÂÂÂÂ int count;
>> ÂÂÂÂÂ int i;
>> ÂÂÂÂÂ int diag, dtab_count;
>> -ÂÂÂ int key;
>> +ÂÂÂ int key, buf_size, ret;
>> Â Â ÂÂÂÂÂ diag = kdbgetintenv("DTABCOUNT", &dtab_count);
>> @@ -336,9 +336,8 @@ static char *kdb_read(char *buffer, size_t bufsize)
>> ÂÂÂÂÂÂÂÂÂ else
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ p_tmp = tmpbuffer;
>> ÂÂÂÂÂÂÂÂÂ len = strlen(p_tmp);
>> -ÂÂÂÂÂÂÂ count = kallsyms_symbol_complete(p_tmp,
>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ sizeof(tmpbuffer) -
>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ (p_tmp - tmpbuffer));
>> +ÂÂÂÂÂÂÂ buf_size = sizeof(tmpbuffer) - (p_tmp - tmpbuffer);
>> +ÂÂÂÂÂÂÂ count = kallsyms_symbol_complete(p_tmp, buf_size);
>> ÂÂÂÂÂÂÂÂÂ if (tab == 2 && count > 0) {
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ kdb_printf("\n%d symbols are found.", count);
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ if (count > dtab_count) {
>> @@ -350,9 +349,13 @@ static char *kdb_read(char *buffer, size_t bufsize)
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ }
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ kdb_printf("\n");
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ for (i = 0; i < count; i++) {
>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (WARN_ON(!kallsyms_symbol_next(p_tmp, i)))
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = kallsyms_symbol_next(p_tmp, i, buf_size);
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (WARN_ON(!ret))
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
> I'm getting confused by having two different branches on ret.
> 
> Don't get a WARN_ON() when ret == -E2BIG?
> 
> 

Should we WARN on a really long symbol?  I don't think we should as we're
handling that by truncating the output and adding ellipses below.

P.

> Daniel.
> 
> 
>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ kdb_printf("%s ", p_tmp);
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (ret != -E2BIG)
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ kdb_printf("%s ", p_tmp);
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ else
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ kdb_printf("%s... ", p_tmp);
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ *(p_tmp + len) = '\0';
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ }
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ if (i >= dtab_count)