2019-05-08 02:13:39

by Wenlin Kang

[permalink] [raw]
Subject: [PATCH] kdb: Fix bound check compiler warning

The strncpy() function may leave the destination string buffer
unterminated, better use strlcpy() instead.

This fixes the following warning with gcc 8.2:

kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Wenlin Kang <[email protected]>
---
kernel/debug/kdb/kdb_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 6a4b414..7fd4513 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
{
if (prompt && kdb_prompt_str != prompt)
- strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
+ strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);
kdb_printf(kdb_prompt_str);
kdb_nextline = 1; /* Prompt and input resets line number */
return kdb_read(buffer, bufsize);
--
1.9.1


2019-05-08 08:57:44

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH] kdb: Fix bound check compiler warning

On Wed, May 08, 2019 at 09:52:39AM +0800, Wenlin Kang wrote:
> The strncpy() function may leave the destination string buffer
> unterminated, better use strlcpy() instead.
>
> This fixes the following warning with gcc 8.2:
>
> kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
> kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
> strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Wenlin Kang <[email protected]>
> ---
> kernel/debug/kdb/kdb_io.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> index 6a4b414..7fd4513 100644
> --- a/kernel/debug/kdb/kdb_io.c
> +++ b/kernel/debug/kdb/kdb_io.c
> @@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
> char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
> {
> if (prompt && kdb_prompt_str != prompt)
> - strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> + strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);

Shouldn't that be strscpy?


Daniel.

> kdb_printf(kdb_prompt_str);
> kdb_nextline = 1; /* Prompt and input resets line number */
> return kdb_read(buffer, bufsize);
> --
> 1.9.1
>

2019-05-09 02:57:34

by Wenlin Kang

[permalink] [raw]
Subject: Re: [PATCH] kdb: Fix bound check compiler warning

On 5/8/19 4:16 PM, Daniel Thompson wrote:
> On Wed, May 08, 2019 at 09:52:39AM +0800, Wenlin Kang wrote:
>> The strncpy() function may leave the destination string buffer
>> unterminated, better use strlcpy() instead.
>>
>> This fixes the following warning with gcc 8.2:
>>
>> kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
>> kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
>> strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Wenlin Kang <[email protected]>
>> ---
>> kernel/debug/kdb/kdb_io.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
>> index 6a4b414..7fd4513 100644
>> --- a/kernel/debug/kdb/kdb_io.c
>> +++ b/kernel/debug/kdb/kdb_io.c
>> @@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
>> char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
>> {
>> if (prompt && kdb_prompt_str != prompt)
>> - strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
>> + strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> Shouldn't that be strscpy?


Hi Daniel

I thought about strscpy, but I think strlcpy is better, because it only
copy the real number of characters if src string less than that size.


>
>
> Daniel.
>
>> kdb_printf(kdb_prompt_str);
>> kdb_nextline = 1; /* Prompt and input resets line number */
>> return kdb_read(buffer, bufsize);
>> --
>> 1.9.1
>>

--
Thanks,
Wenlin Kang

2019-05-12 09:02:40

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH] kdb: Fix bound check compiler warning

On Thu, May 09, 2019 at 10:56:03AM +0800, Wenlin Kang wrote:
> On 5/8/19 4:16 PM, Daniel Thompson wrote:
> > On Wed, May 08, 2019 at 09:52:39AM +0800, Wenlin Kang wrote:
> > > The strncpy() function may leave the destination string buffer
> > > unterminated, better use strlcpy() instead.
> > >
> > > This fixes the following warning with gcc 8.2:
> > >
> > > kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
> > > kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
> > > strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Signed-off-by: Wenlin Kang <[email protected]>
> > > ---
> > > kernel/debug/kdb/kdb_io.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> > > index 6a4b414..7fd4513 100644
> > > --- a/kernel/debug/kdb/kdb_io.c
> > > +++ b/kernel/debug/kdb/kdb_io.c
> > > @@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
> > > char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
> > > {
> > > if (prompt && kdb_prompt_str != prompt)
> > > - strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > > + strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > Shouldn't that be strscpy?
>
>
> Hi Daniel
>
> I thought about strscpy, but I think strlcpy is better, because it only copy
> the real number of characters if src string less than that size.

Sorry, I'm confused by this. What behavior does strscpy() have that you
consider undesirable in this case?


Daniel.

2019-05-13 04:34:40

by Wenlin Kang

[permalink] [raw]
Subject: Re: [PATCH] kdb: Fix bound check compiler warning

On 5/12/19 5:00 PM, Daniel Thompson wrote:
> On Thu, May 09, 2019 at 10:56:03AM +0800, Wenlin Kang wrote:
>> On 5/8/19 4:16 PM, Daniel Thompson wrote:
>>> On Wed, May 08, 2019 at 09:52:39AM +0800, Wenlin Kang wrote:
>>>> The strncpy() function may leave the destination string buffer
>>>> unterminated, better use strlcpy() instead.
>>>>
>>>> This fixes the following warning with gcc 8.2:
>>>>
>>>> kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
>>>> kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
>>>> strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>
>>>> Signed-off-by: Wenlin Kang <[email protected]>
>>>> ---
>>>> kernel/debug/kdb/kdb_io.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
>>>> index 6a4b414..7fd4513 100644
>>>> --- a/kernel/debug/kdb/kdb_io.c
>>>> +++ b/kernel/debug/kdb/kdb_io.c
>>>> @@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
>>>> char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
>>>> {
>>>> if (prompt && kdb_prompt_str != prompt)
>>>> - strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
>>>> + strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);
>>> Shouldn't that be strscpy?
>>
>> Hi Daniel
>>
>> I thought about strscpy, but I think strlcpy is better, because it only copy
>> the real number of characters if src string less than that size.
> Sorry, I'm confused by this. What behavior does strscpy() have that you
> consider undesirable in this case?


Hi Daniel

I checked strscpy() again, and think either is fine to me, if you think
strscpy() is better, I can change it to this, and send v2, thanks for
your review.


>
> Daniel.
>

--
Thanks,
Wenlin Kang

2019-05-13 08:17:55

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH] kdb: Fix bound check compiler warning

On Mon, May 13, 2019 at 11:39:47AM +0800, Wenlin Kang wrote:
> On 5/12/19 5:00 PM, Daniel Thompson wrote:
> > On Thu, May 09, 2019 at 10:56:03AM +0800, Wenlin Kang wrote:
> > > On 5/8/19 4:16 PM, Daniel Thompson wrote:
> > > > On Wed, May 08, 2019 at 09:52:39AM +0800, Wenlin Kang wrote:
> > > > > The strncpy() function may leave the destination string buffer
> > > > > unterminated, better use strlcpy() instead.
> > > > >
> > > > > This fixes the following warning with gcc 8.2:
> > > > >
> > > > > kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
> > > > > kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
> > > > > strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >
> > > > > Signed-off-by: Wenlin Kang <[email protected]>
> > > > > ---
> > > > > kernel/debug/kdb/kdb_io.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> > > > > index 6a4b414..7fd4513 100644
> > > > > --- a/kernel/debug/kdb/kdb_io.c
> > > > > +++ b/kernel/debug/kdb/kdb_io.c
> > > > > @@ -446,7 +446,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
> > > > > char *kdb_getstr(char *buffer, size_t bufsize, const char *prompt)
> > > > > {
> > > > > if (prompt && kdb_prompt_str != prompt)
> > > > > - strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > > > > + strlcpy(kdb_prompt_str, prompt, CMD_BUFLEN);
> > > > Shouldn't that be strscpy?
> > >
> > > Hi Daniel
> > >
> > > I thought about strscpy, but I think strlcpy is better, because it only copy
> > > the real number of characters if src string less than that size.
> > Sorry, I'm confused by this. What behavior does strscpy() have that you
> > consider undesirable in this case?
>
>
> Hi Daniel
>
> I checked strscpy() again, and think either is fine to me, if you think
> strscpy() is better, I can change it to this, and send v2, thanks for your
> review.

I think strscpy() is better.


Daniel.

>
>
> >
> > Daniel.
> >
>
> --
> Thanks,
> Wenlin Kang
>