This allows access to an element after the end of the array. Fix it.

Reported-by: Coverity (CID: 173279) Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Change log_get_cat_name() to always return a valid string, even if the uclass is not found

common/log.c | 10 +++++++--- include/log.h | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/common/log.c b/common/log.c index 3b5588ebe7..59869cd29d 100644 --- a/common/log.c +++ b/common/log.c @@ -38,12 +38,16 @@ static const char *log_level_name[LOGL_COUNT] = {

const char *log_get_cat_name(enum log_category_t cat) { - if (cat > LOGC_COUNT) - return "invalid"; + const char *name; + + if (cat < 0 || cat >= LOGC_COUNT) + return "<invalid>"; if (cat >= LOGC_NONE) return log_cat_name[cat - LOGC_NONE];

- return uclass_get_name((enum uclass_id)cat); + name = uclass_get_name((enum uclass_id)cat); + + return name ? name : "<missing>"; }

enum log_category_t log_get_cat_by_name(const char *name) diff --git a/include/log.h b/include/log.h index a3edd25546..3e99d6e62b 100644 --- a/include/log.h +++ b/include/log.h @@ -274,7 +274,8 @@ struct log_filter { * log_get_cat_name() - Get the name of a category * * @cat: Category to look up - * @return category name (which may be a uclass driver name) + * @return category name (which may be a uclass driver name) if found, or + * "<invalid>" if invalid, or "<missing>" if not found */ const char *log_get_cat_name(enum log_category_t cat);

Both putc() and puts() can be called before global_data is set up. Some of the code paths don't handle this correctly. Add an explicit test before any member is accessed.

Reported-by: Coverity (CID: 169030) Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de ---

Changes in v2: None

common/console.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/common/console.c b/common/console.c index 2688af56e1..2ba33dc574 100644 --- a/common/console.c +++ b/common/console.c @@ -502,8 +502,10 @@ void putc(const char c) return; } #endif + if (!gd) + return; #ifdef CONFIG_CONSOLE_RECORD - if (gd && (gd->flags & GD_FLG_RECORD) && gd->console_out.start) + if ((gd->flags & GD_FLG_RECORD) && gd->console_out.start) membuff_putbyte(&gd->console_out, c); #endif #ifdef CONFIG_SILENT_CONSOLE @@ -541,8 +543,10 @@ void puts(const char *s) return; } #endif + if (!gd) + return; #ifdef CONFIG_CONSOLE_RECORD - if (gd && (gd->flags & GD_FLG_RECORD) && gd->console_out.start) + if ((gd->flags & GD_FLG_RECORD) && gd->console_out.start) membuff_put(&gd->console_out, s, strlen(s)); #endif #ifdef CONFIG_SILENT_CONSOLE

The use of strcpy() to remove characters at the start of a string is safe in U-Boot, since we know the implementation. But in os.c we are using the C library's strcpy() function, where this behaviour is not permitted.

Update the code to use memcpy() instead.

Reported-by: Coverity (CID: 173279) Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Also remove the leading / from the "/spl" path

arch/sandbox/cpu/os.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 5839932b00..5a12b8c677 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -585,9 +585,10 @@ int os_find_u_boot(char *fname, int maxlen) }

/* Look for 'u-boot' in the parent directory of spl/ */ - p = strstr(fname, "/spl/"); + p = strstr(fname, "spl/"); if (p) { - strcpy(p, p + 4); + /* Remove the "spl" characters */ + memmove(p, p + 3, strlen(p + 3) + 1); fd = os_open(fname, O_RDONLY); if (fd >= 0) { close(fd);

This loop never actually exits, but the way the code is written this is not obvious. Add an explicit error check.

Reported-by: Coverity (CID: 131280) Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add a missing free() from one error path

tools/fdtgrep.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index f2b8b71ed7..d9f6fb0740 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -801,7 +801,7 @@ static int do_fdtgrep(struct display_info *disp, const char *filename) * The first pass will count the regions, but if it is too many, * we do another pass to actually record them. */ - for (i = 0; i < 3; i++) { + for (i = 0; i < 2; i++) { region = malloc(count * sizeof(struct fdt_region)); if (!region) { fprintf(stderr, "Out of memory for %d regions\n", @@ -815,11 +815,14 @@ static int do_fdtgrep(struct display_info *disp, const char *filename) disp->flags); if (count < 0) { report_error("fdt_find_regions", count); + free(region); return -1; } if (count <= max_regions) break; free(region); + fprintf(stderr, "Internal error with fdtgrep_find_region)(\n"); + return -1; }

/* Optionally print a list of regions */

The current code might succeed on the first allocation and fail on the second. Separate the checks to avoid this problem.

Of course, free() will never fail and the chances that (when allocating two small areas) one will succeed and one will fail are just as remote. But this keeps coverity happy.

Reported-by: Coverity (CID: 131226)

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

tools/fdtgrep.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index d9f6fb0740..99f955b629 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -133,11 +133,11 @@ static int value_add(struct display_info *disp, struct value_node **headp, }

str = strdup(str); + if (!str) + goto err_mem; node = malloc(sizeof(*node)); - if (!str || !node) { - fprintf(stderr, "Out of memory\n"); - return -1; - } + if (!node) + goto err_mem; node->next = *headp; node->type = type; node->include = include; @@ -145,6 +145,9 @@ static int value_add(struct display_info *disp, struct value_node **headp, *headp = node;

return 0; +err_mem: + fprintf(stderr, "Out of memory\n"); + return -1; }

static bool util_is_printable_string(const void *data, int len)

Thsi function can fail without freeing all its memory. Fix it.

Reported-by: Coverity (CID: 131217) Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add a comment defending the technical memory leaks that remain

lib/rsa/rsa-sign.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index d2788bf79a..cfe09cc94c 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -635,6 +635,15 @@ static int fdt_add_bignum(void *blob, int noffset, const char *prop_name, big2 = BN_new(); big32 = BN_new(); big2_32 = BN_new(); + + /* + * Note: This code assumes that all of the above succeed, or all fail. + * In practice memory allocations generally do not fail (unless the + * process is killed), so it does not seem worth handling each of these + * as a separate case. Technicaly this could leak memory on failure, + * but a) it won't happen in practice, and b) it doesn't matter as we + * will immediately exit with a failure code. + */ if (!tmp || !big2 || !big32 || !big2_32) { fprintf(stderr, "Out of memory (bignum)\n"); return -ENOMEM; @@ -667,15 +676,13 @@ static int fdt_add_bignum(void *blob, int noffset, const char *prop_name, * might fail several times */ ret = fdt_setprop(blob, noffset, prop_name, buf, size); - if (ret) - return -FDT_ERR_NOSPACE; free(buf); BN_free(tmp); BN_free(big2); BN_free(big32); BN_free(big2_32);

- return ret; + return ret ? -FDT_ERR_NOSPACE : 0; }

int rsa_add_verify_data(struct image_sign_info *info, void *keydest)

Move the strdup() call so that it is only done when we know we will bind the device.

Reported-by: Coverity (CID: 131216)

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Fre the string when device_bind() fails, and presumably doesn't need it

drivers/mtd/spi/sandbox.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c index 7893efee12..f23c0e13e0 100644 --- a/drivers/mtd/spi/sandbox.c +++ b/drivers/mtd/spi/sandbox.c @@ -567,16 +567,17 @@ int sandbox_sf_bind_emul(struct sandbox_state *state, int busnum, int cs, strncpy(name, spec, sizeof(name) - 6); name[sizeof(name) - 6] = '\0'; strcat(name, "-emul"); - str = strdup(name); - if (!str) - return -ENOMEM; drv = lists_driver_lookup_name("sandbox_sf_emul"); if (!drv) { puts("Cannot find sandbox_sf_emul driver\n"); return -ENOENT; } + str = strdup(name); + if (!str) + return -ENOMEM; ret = device_bind(bus, drv, str, NULL, of_offset, &emul); if (ret) { + free(str); printf("Cannot create emul device for spec '%s' (err=%d)\n", spec, ret); return ret;

At present the code overruns the bar[] array. Fix this.

At the same time, drop the leading / from the "/spl" path so that we can run U-Boot SPL with:

spl/u-boot-spl

rather than requiring:

/path/to/spl/u-boot-spl

Reported-by: Coverity (CID: 131199)

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de ---

Changes in v2: None

drivers/misc/swap_case.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c index 56cd0700fa..b777404c09 100644 --- a/drivers/misc/swap_case.c +++ b/drivers/misc/swap_case.c @@ -21,7 +21,7 @@ */ struct swap_case_platdata { u16 command; - u32 bar[2]; + u32 bar[6]; };

#define offset_to_barnum(offset) \