| Message ID | 1452346308-16676-1-git-send-email-van.freenix@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
On Sat, Jan 09, 2016 at 09:31:48PM +0800, Peng Fan wrote: > Report Coverity log: > Destination buffer too small (STRING_OVERFLOW) > string_overflow: You might overrun the 1024 byte destination string > lastcommand by writing 1025 bytes from console_buffer > > Signed-off-by: Peng Fan <van.freenix@gmail.com> > Cc: Heiko Schocher <hs@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@konsulko.com> > --- > common/cli_simple.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/common/cli_simple.c b/common/cli_simple.c > index 9c3d073..c51f963 100644 > --- a/common/cli_simple.c > +++ b/common/cli_simple.c > @@ -276,7 +276,8 @@ void cli_simple_loop(void) > > flag = 0; /* assume no special flags for now */ > if (len > 0) > - strcpy(lastcommand, console_buffer); > + strncpy(lastcommand, console_buffer, > + CONFIG_SYS_CBSIZE + 1); > else if (len == 0) > flag |= CMD_FLAG_REPEAT; > #ifdef CONFIG_BOOT_RETRY_TIME So, long term I would like to see use move to using strlcpy for the normal case (it might not make sense when working with various defined protocols, etc). Thanks!
On Sat, Jan 09, 2016 at 09:03:59AM -0500, Tom Rini wrote: >On Sat, Jan 09, 2016 at 09:31:48PM +0800, Peng Fan wrote: > >> Report Coverity log: >> Destination buffer too small (STRING_OVERFLOW) >> string_overflow: You might overrun the 1024 byte destination string >> lastcommand by writing 1025 bytes from console_buffer >> >> Signed-off-by: Peng Fan <van.freenix@gmail.com> >> Cc: Heiko Schocher <hs@denx.de> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Tom Rini <trini@konsulko.com> >> --- >> common/cli_simple.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/common/cli_simple.c b/common/cli_simple.c >> index 9c3d073..c51f963 100644 >> --- a/common/cli_simple.c >> +++ b/common/cli_simple.c >> @@ -276,7 +276,8 @@ void cli_simple_loop(void) >> >> flag = 0; /* assume no special flags for now */ >> if (len > 0) >> - strcpy(lastcommand, console_buffer); >> + strncpy(lastcommand, console_buffer, >> + CONFIG_SYS_CBSIZE + 1); >> else if (len == 0) >> flag |= CMD_FLAG_REPEAT; >> #ifdef CONFIG_BOOT_RETRY_TIME > >So, long term I would like to see use move to using strlcpy for the >normal case (it might not make sense when working with various defined >protocols, etc). Thanks! Thanks. Just read this, https://www.sudo.ws/todd/papers/strlcpy.html. strlcpy is a better choice. Thanks, Peng. > >-- >Tom
diff --git a/common/cli_simple.c b/common/cli_simple.c index 9c3d073..c51f963 100644 --- a/common/cli_simple.c +++ b/common/cli_simple.c @@ -276,7 +276,8 @@ void cli_simple_loop(void) flag = 0; /* assume no special flags for now */ if (len > 0) - strcpy(lastcommand, console_buffer); + strncpy(lastcommand, console_buffer, + CONFIG_SYS_CBSIZE + 1); else if (len == 0) flag |= CMD_FLAG_REPEAT; #ifdef CONFIG_BOOT_RETRY_TIME
Report Coverity log: Destination buffer too small (STRING_OVERFLOW) string_overflow: You might overrun the 1024 byte destination string lastcommand by writing 1025 bytes from console_buffer Signed-off-by: Peng Fan <van.freenix@gmail.com> Cc: Heiko Schocher <hs@denx.de> Cc: Simon Glass <sjg@chromium.org> Cc: Tom Rini <trini@konsulko.com> --- common/cli_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)