On Fri, Oct 28, 2016 at 12:41:05PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On 28 October 2016 at 11:59, Tom Rini <tr...@konsulko.com> wrote:
> > On Thu, Oct 27, 2016 at 08:18:39PM -0600, Simon Glass wrote:
> >> Coverity complains that this can overflow. If we later increase the size
> >> of one of the strings in the table, it could happen.
> >>
> >> Adjust the code to protect against this.
> >>
> >> Signed-off-by: Simon Glass <s...@chromium.org>
> >> Reported-by: Coverity (CID: 150964)
> >> ---
> >>
> >> Changes in v3:
> >> - Adjust to deal with what strncpy() actually does (I think)
> >>
> >> Changes in v2:
> >> - Drop unwanted #include
> >>
> >> common/image.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/image.c b/common/image.c
> >> index 0e86c13..016f263 100644
> >> --- a/common/image.c
> >> +++ b/common/image.c
> >> @@ -588,9 +588,11 @@ const table_entry_t *get_table_entry(const
> >> table_entry_t *table, int id)
> >> static const char *unknown_msg(enum ih_category category)
> >> {
> >> static char msg[30];
> >> + static char unknown_str = "Unknown ";
> >>
> >> - strcpy(msg, "Unknown ");
> >> - strcat(msg, table_info[category].desc);
> >> + strcpy(msg, unknown_str);
> >> + strncat(msg, table_info[category].desc,
> >> + sizeof(msg) - sizeof(unknown_str));
> >
> > We still need to subtract 1 more here at the end, for the NUL don't we?
>
> I was hoping that the sizeof(msg) would take care of that?
No, and you didn't compile test this did you? ;) I was trying to throw
up a stupid test to confirm what all everything would be.
test.c:6:28: warning: initialization makes integer from pointer without a cast
[-Wint-conversion]
static char unknown_str = "Unknown ";
^~~~~~~~~~
test.c:6:28: error: initializer element is not computable at load time
Correcting that to be *unknown_str gives us back the sizeof(char *) not
the string in question. So we need to use strlen(unknown_str), and
strlen does not include the NUL, so we would need to still add in the -
1 after all of the above.
And I'm being verbose above because string handling can be annoying and
I didn't get it 100% right in my head so I figured it's worth showing
the work. And since we're going with "Coverity says we've got string
problems" we should really correct it, and not just be wrong in a
different way :)
--
Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
