September 2017 - U-Boot

[U-Boot] [PATCH] tools/fit_image.c: Update some return code paths
by Tom Rini 07 Oct '17

07 Oct '17

Coverity has found some problems with the return paths in parts of this code. We have a case where we were going to the wrong part of the unwind (open() failed so we cannot close the fd), a case where we were only free()ing our buf on the error path and finally a case where we did not munmap in the failure path. Reported-by: Coverity (CID: 138492, 138495, 143064) Signed-off-by: Tom Rini <trini(a)konsulko.com> --- tools/fit_image.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/fit_image.c b/tools/fit_image.c index 4dc8bd886245..c6026567f3a6 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -372,7 +372,7 @@ static int fit_build(struct image_tool_params *params, const char *fname) if (fd < 0) { fprintf(stderr, "%s: Can't open %s: %s\n", params->cmdname, fname, strerror(errno)); - goto err; + goto err_buf; } ret = write(fd, buf, size); if (ret != size) { @@ -501,6 +501,7 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname) ret = -EIO; goto err; } + free(buf); close(fd); return 0; @@ -601,6 +602,7 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) ret = 0; err: + munmap(old_fdt, sbuf.st_size); free(fdt); close(fd); return ret; -- 1.9.1

1 1

[U-Boot] [PATCH] cmd/time.c: Initialize 'repeatable' variable
by Tom Rini 07 Oct '17

07 Oct '17

We cannot leave this uninitialized, set it to 0. Reported-by: Coverity (CID: 144426) Signed-off-by: Tom Rini <trini(a)konsulko.com> --- cmd/time.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/time.c b/cmd/time.c index de57e3b9dd5e..2cd8b1a57777 100644 --- a/cmd/time.c +++ b/cmd/time.c @@ -28,7 +28,7 @@ static int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong cycles = 0; int retval = 0; - int repeatable; + int repeatable = 0; if (argc == 1) return CMD_RET_USAGE; -- 1.9.1

1 1

[U-Boot] [PATCH] cmd/pxe.c: Rework initrd and bootargs handling slightly
by Tom Rini 07 Oct '17

07 Oct '17

For the initrd portion of handling our bootm arguments we do not have a sufficiently long enough buffer for some improbable 64bit cases. Expand this buffer to allow for a 64bit address and almost 256MB initrd to be used. Make use of strncpy/strncat when constructing the values here since we know what the worst case valid values are, length wise. Similarly for bootargs themselves, we need to make use of strlen/sizeof and strncpy/strncat to ensure that we don't overflow bootargs itself. Cc: Simon Glass <sjg(a)chromium.org> Cc: Alexander Graf <agraf(a)suse.de> Reported-by: Coverity (CID: 131256) Signed-off-by: Tom Rini <trini(a)konsulko.com> --- cmd/pxe.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/pxe.c b/cmd/pxe.c index c5a770a26995..a62cbe192a32 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -616,7 +616,7 @@ static int label_localboot(struct pxe_label *label) static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) { char *bootm_argv[] = { "bootm", NULL, NULL, NULL, NULL }; - char initrd_str[22]; + char initrd_str[28]; char mac_str[29] = ""; char ip_str[68] = ""; int bootm_argc = 2; @@ -648,9 +648,9 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) } bootm_argv[2] = initrd_str; - strcpy(bootm_argv[2], env_get("ramdisk_addr_r")); + strncpy(bootm_argv[2], env_get("ramdisk_addr_r"), 18); strcat(bootm_argv[2], ":"); - strcat(bootm_argv[2], env_get("filesize")); + strncat(bootm_argv[2], env_get("filesize"), 9); } if (get_relfile_envaddr(cmdtp, label->kernel, "kernel_addr_r") < 0) { @@ -689,9 +689,9 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) } if (label->append) - strcpy(bootargs, label->append); - strcat(bootargs, ip_str); - strcat(bootargs, mac_str); + strncpy(bootargs, label->append, sizeof(bootargs)); + strncat(bootargs, ip_str, sizeof(bootargs) - strlen(bootargs)); + strncat(bootargs, mac_str, sizeof(bootargs) - strlen(bootargs)); cli_simple_process_macros(bootargs, finalbootargs); env_set("bootargs", finalbootargs); -- 1.9.1

1 1

[U-Boot] [PATCH] cmd/nvedit.c: Update input handling to cover overflow cases
by Tom Rini 07 Oct '17

07 Oct '17

When we have multiple messages provided, we need to be sure that we do not exceed the length of our 'message' buffer. In the for loop, make sure that pos is not larger than message. Only copy in at most however much of the message buffer remains. Finally, if we have not reached the end of the message buffer, put in a space and NULL, and if we have, ensure the buffer is now NULL termined. Reported-by: Coverity (CID: 165116) Signed-off-by: Tom Rini <trini(a)konsulko.com> --- cmd/nvedit.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 4033d90c8e2d..055836cc72d1 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -393,15 +393,18 @@ int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) sprintf(message, "Please enter '%s': ", argv[1]); } else { /* env_ask envname message1 ... messagen [size] */ - for (i = 2, pos = 0; i < argc; i++) { + for (i = 2, pos = 0; i < argc && pos < sizeof(message); i++) { if (pos) message[pos++] = ' '; - strcpy(message + pos, argv[i]); + strncpy(message + pos, argv[i], sizeof(message) - pos); pos += strlen(argv[i]); } - message[pos++] = ' '; - message[pos] = '\0'; + if (pos < sizeof(message) - 1) { + message[pos++] = ' '; + message[pos] = '\0'; + } else + message[CONFIG_SYS_CBSIZE - 1] = '\0'; } if (size >= CONFIG_SYS_CBSIZE) -- 1.9.1

1 1

[U-Boot] [PATCH] test/dm: Fix string handling issues in the eth test
by Tom Rini 07 Oct '17

07 Oct '17

Coverity scan has identified potential buffer overruns in these tests. Correct this by zeroing our buffer and using strncpy not strcpy. Reported-by: Coverity (CID: 155462, 155463) Cc: Joe Hershberger <joe.hershberger(a)ni.com> Cc: Simon Glass <sjg(a)chromium.org> Cc: Bin Meng <bmeng.cn(a)gmail.com> Signed-off-by: Tom Rini <trini(a)konsulko.com> --- test/dm/eth.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/dm/eth.c b/test/dm/eth.c index 122fab924d6a..67fd660ee4f5 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -110,6 +110,7 @@ static int dm_test_eth_act(struct unit_test_state *uts) char ethaddr[DM_TEST_ETH_NUM][18]; int i; + memset(ethaddr, '\0', sizeof(ethaddr)); net_ping_ip = string_to_ip("1.1.2.2"); /* Prepare the test scenario */ @@ -119,7 +120,7 @@ static int dm_test_eth_act(struct unit_test_state *uts) ut_assertok(device_remove(dev[i], DM_REMOVE_NORMAL)); /* Invalidate MAC address */ - strcpy(ethaddr[i], env_get(addrname[i])); + strncpy(ethaddr[i], env_get(addrname[i]), 17); /* Must disable access protection for ethaddr before clearing */ env_set(".flags", addrname[i]); env_set(addrname[i], NULL); @@ -187,7 +188,8 @@ static int dm_test_eth_rotate(struct unit_test_state *uts) net_ping_ip = string_to_ip("1.1.2.2"); /* Invalidate eth1's MAC address */ - strcpy(ethaddr, env_get("eth1addr")); + memset(ethaddr, '\0', sizeof(ethaddr)); + strncpy(ethaddr, env_get("eth1addr"), 17); /* Must disable access protection for eth1addr before clearing */ env_set(".flags", "eth1addr"); env_set("eth1addr", NULL); @@ -200,7 +202,7 @@ static int dm_test_eth_rotate(struct unit_test_state *uts) if (!retval) { /* Invalidate eth0's MAC address */ - strcpy(ethaddr, env_get("ethaddr")); + strncpy(ethaddr, env_get("ethaddr"), 17); /* Must disable access protection for ethaddr before clearing */ env_set(".flags", "ethaddr"); env_set("ethaddr", NULL); -- 1.9.1

3 3

[U-Boot] [PATCH] test/overlay: Fix various malloc/free leaks
by Tom Rini 07 Oct '17

07 Oct '17

With the overlay tests now being built in sandbox Coverity has found a number of issues in the tests. In short, if malloc ever failed we would leak the previous mallocs, so we need to do the usual goto pattern to free each in turn. Finally, we always looked at the free()d location to see how many tests had failed for the return code. Reported-by: Coverity (CID: 167224, 167227, 167230, 167236) Signed-off-by: Tom Rini <trini(a)konsulko.com> --- test/overlay/cmd_ut_overlay.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/overlay/cmd_ut_overlay.c b/test/overlay/cmd_ut_overlay.c index 24891ee82901..c730a11f5188 100644 --- a/test/overlay/cmd_ut_overlay.c +++ b/test/overlay/cmd_ut_overlay.c @@ -226,6 +226,7 @@ int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) void *fdt_overlay = &__dtb_test_fdt_overlay_begin; void *fdt_overlay_stacked = &__dtb_test_fdt_overlay_stacked_begin; void *fdt_base_copy, *fdt_overlay_copy, *fdt_overlay_stacked_copy; + int ret = -ENOMEM; uts = calloc(1, sizeof(*uts)); if (!uts) @@ -236,16 +237,16 @@ int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) fdt_base_copy = malloc(FDT_COPY_SIZE); if (!fdt_base_copy) - return -ENOMEM; + goto err1; uts->priv = fdt_base_copy; fdt_overlay_copy = malloc(FDT_COPY_SIZE); if (!fdt_overlay_copy) - return -ENOMEM; + goto err2; fdt_overlay_stacked_copy = malloc(FDT_COPY_SIZE); if (!fdt_overlay_stacked_copy) - return -ENOMEM; + goto err3; /* * Resize the FDT to 4k so that we have room to operate on @@ -293,11 +294,18 @@ int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } printf("Failures: %d\n", uts->fail_count); + if (!uts->fail_count) + ret = 0; + else + ret = CMD_RET_FAILURE; free(fdt_overlay_stacked_copy); +err3: free(fdt_overlay_copy); +err2: free(fdt_base_copy); +err1: free(uts); - return uts->fail_count ? CMD_RET_FAILURE : 0; + return ret; } -- 1.9.1

2 3

[U-Boot] [PATCH] GPT: fix memory leaks identified by Coverity
by alison＠peloton-tech.com 07 Oct '17

07 Oct '17

From: Alison Chaiken <alison(a)she-devel.com> Create a common exit for most of the error handling code in do_rename_gpt_parts. Delete the list elements in disk_partitions before calling INIT_LIST_HEAD from get_gpt_info() a second time. The SIZEOF_MISMATCH error is not addressed, since that problem was already fixed by "GPT: incomplete initialization in allocate_disk_part". Signed-off-by: Alison Chaiken <alison(a)peloton-tech.com> --- cmd/gpt.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/cmd/gpt.c b/cmd/gpt.c index d4406e3120..dfa41947e1 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -633,6 +633,21 @@ static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr) } #ifdef CONFIG_CMD_GPT_RENAME + +/* There are 3 malloc() calls in set_gpt_info() and there is no info about which + * failed. + */ +static void set_gpt_cleanup(char **str_disk_guid, + disk_partition_t **partitions) +{ +#ifdef CONFIG_RANDOM_UUID + if (str_disk_guid) + free(str_disk_guid); +#endif + if (partitions) + free(partitions); +} + static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm, char *name1, char *name2) { @@ -651,19 +666,26 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm, ret = get_disk_guid(dev_desc, disk_guid); if (ret < 0) return ret; + /* Allocates disk_partitions, requiring matching call to del_gpt_info() + * if successful. + */ numparts = get_gpt_info(dev_desc); if (numparts <= 0) return numparts ? numparts : -ENODEV; partlistlen = calc_parts_list_len(numparts); partitions_list = malloc(partlistlen); - if (partitions_list == NULL) + if (!partitions_list) { + del_gpt_info(); return -ENOMEM; + } memset(partitions_list, '\0', partlistlen); ret = create_gpt_partitions_list(numparts, disk_guid, partitions_list); - if (ret < 0) + if (ret < 0) { + free(partitions_list); return ret; + } /* * Uncomment the following line to print a string that 'gpt write' * or 'gpt verify' will accept as input. @@ -671,15 +693,23 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm, debug("OLD partitions_list is %s with %u chars\n", partitions_list, (unsigned)strlen(partitions_list)); + /* set_gpt_info allocates new_partitions and str_disk_guid */ ret = set_gpt_info(dev_desc, partitions_list, &str_disk_guid, &new_partitions, &part_count); - if (ret < 0) - return ret; + if (ret < 0) { + del_gpt_info(); + free(partitions_list); + if (ret == -ENOMEM) + set_gpt_cleanup(&str_disk_guid, &new_partitions); + else + goto out; + } if (!strcmp(subcomm, "swap")) { if ((strlen(name1) > PART_NAME_LEN) || (strlen(name2) > PART_NAME_LEN)) { printf("Names longer than %d characters are truncated.\n", PART_NAME_LEN); - return -EINVAL; + ret = -EINVAL; + goto out; } list_for_each(pos, &disk_partitions) { curr = list_entry(pos, struct disk_part, list); @@ -693,21 +723,24 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm, } if ((ctr1 + ctr2 < 2) || (ctr1 != ctr2)) { printf("Cannot swap partition names except in pairs.\n"); - return -EINVAL; + ret = -EINVAL; + goto out; } } else { /* rename */ if (strlen(name2) > PART_NAME_LEN) { printf("Names longer than %d characters are truncated.\n", PART_NAME_LEN); - return -EINVAL; + ret = -EINVAL; + goto out; } partnum = (int)simple_strtol(name1, NULL, 10); if ((partnum < 0) || (partnum > numparts)) { printf("Illegal partition number %s\n", name1); - return -EINVAL; + ret = -EINVAL; + goto out; } ret = part_get_info(dev_desc, partnum, new_partitions); if (ret < 0) - return ret; + goto out; /* U-Boot partition numbering starts at 1 */ list_for_each(pos, &disk_partitions) { @@ -722,33 +755,48 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm, ret = create_gpt_partitions_list(numparts, disk_guid, partitions_list); if (ret < 0) - return ret; + goto out; debug("NEW partitions_list is %s with %u chars\n", partitions_list, (unsigned)strlen(partitions_list)); ret = set_gpt_info(dev_desc, partitions_list, &str_disk_guid, &new_partitions, &part_count); - if (ret < 0) - return ret; + /* Even though valid pointers are here passed into set_gpt_info(), + * it mallocs again, and there's no way to tell which failed. + */ + if (ret < 0) { + del_gpt_info(); + free(partitions_list); + if (ret == -ENOMEM) + set_gpt_cleanup(&str_disk_guid, &new_partitions); + else + goto out; + } debug("Writing new partition table\n"); ret = gpt_restore(dev_desc, disk_guid, new_partitions, numparts); if (ret < 0) { printf("Writing new partition table failed\n"); - return ret; + goto out; } debug("Reading back new partition table\n"); + /* Empty the existing disk_partitions list, as otherwise the memory in + * the original list is unreachable. + */ + del_gpt_info(); numparts = get_gpt_info(dev_desc); - if (numparts <= 0) - return numparts ? numparts : -ENODEV; + if (numparts <= 0) { + ret = numparts ? numparts : -ENODEV; + goto out; + } printf("new partition table with %d partitions is:\n", numparts); print_gpt_info(); - del_gpt_info(); - free(partitions_list); - free(str_disk_guid); + out: free(new_partitions); + free(str_disk_guid); + free(partitions_list); return ret; } #endif -- 2.14.1

4 6

[U-Boot] [PATCH v2 1/4] fs/ext4: Fix group descriptor checksum calculation
by Tuomas Tynkkynen 07 Oct '17

07 Oct '17

The current code doesn't compute the group descriptor checksum correctly for the filesystems that e2fsprogs 1.43.4 creates (they have 'Group descriptor size: 64' as reported by tune2fs). Extend the checksum calculation to be done as ext4_group_desc_csum() does in Linux. This fixes these errors in dmesg from running fs-test.sh and makes it succeed again: [1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) [1671902.620706] EXT4-fs (loop1): group descriptors corrupted! Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen(a)iki.fi> --- v2: New patch --- fs/ext4/ext4_common.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 621c61e5c7..31952f48b9 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -432,6 +432,10 @@ uint16_t ext4fs_checksum_update(uint32_t i) crc = ext2fs_crc16(crc, desc, offset); offset += sizeof(desc->bg_checksum); /* skip checksum */ assert(offset == sizeof(*desc)); + if (offset < fs->gdsize) { + crc = ext2fs_crc16(crc, (__u8 *)desc + offset, + fs->gdsize - offset); + } } return crc; -- 2.13.0

2 12

[U-Boot] [PATCH] doc: Fix statements on case sensitivity for regulator prefix
by Felix Brack 07 Oct '17

07 Oct '17

The regulator bindings state that regulator prefixes are allowd to be in upper or lower case. However pmic_bind_children from pmic_uclass uses strncmp to compare DT node name against prefix. This comparison is case sensitive hence the regulator driver prefix case matters. Signed-off-by: Felix Brack <fb(a)ltec.ch> --- doc/device-tree-bindings/regulator/regulator.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt index 2cf4b9d..918711e 100644 --- a/doc/device-tree-bindings/regulator/regulator.txt +++ b/doc/device-tree-bindings/regulator/regulator.txt @@ -10,10 +10,10 @@ pmic: drivers/power/pmic/max77686.c regulator: drivers/power/regulator/max77686.c For the node name e.g.: "prefix[:alpha:]num { ... }": -- the driver prefix should be: "prefix" or "PREFIX" - case insensitive +- the driver prefix should be: "prefix" - case sensitive - the node name's "num" is set as "dev->driver_data" on bind -Example the prefix "ldo" will pass for: "ldo1", "ldo@1", "LDO1", "LDOREG@1"... +Example the prefix "ldo" will pass for: "ldo1", "ldo@1", "ldoreg@1, ... Optional properties: - regulator-name: a string, required by the regulator uclass -- 2.7.4

2 1

[U-Boot] [PATCH 1/2] disk: Provide API to get partition by name for specific type
by Sam Protsenko 07 Oct '17

07 Oct '17

There is already existing function part_get_info_by_name(). But sometimes user is particularly interested in looking for only specific partition type. This patch implements such an API that provides partition searching by name for specified partition type. Signed-off-by: Sam Protsenko <semen.protsenko(a)linaro.org> --- disk/part.c | 15 +++++++++++++-- include/part.h | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/disk/part.c b/disk/part.c index aa9183d696..66b8101f98 100644 --- a/disk/part.c +++ b/disk/part.c @@ -21,6 +21,9 @@ #define PRINTF(fmt,args...) #endif +/* Check all partition types */ +#define PART_TYPE_ALL -1 + DECLARE_GLOBAL_DATA_PTR; #ifdef HAVE_BLOCK_DEVICE @@ -626,8 +629,8 @@ cleanup: return ret; } -int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, - disk_partition_t *info) +int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name, + disk_partition_t *info, int part_type) { struct part_driver *first_drv = ll_entry_start(struct part_driver, part_driver); @@ -638,6 +641,8 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, int ret; int i; for (i = 1; i < part_drv->max_entries; i++) { + if (part_type >= 0 && part_type != part_drv->part_type) + break; ret = part_drv->get_info(dev_desc, i, info); if (ret != 0) { /* no more entries in table */ @@ -652,6 +657,12 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, return -1; } +int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, + disk_partition_t *info) +{ + return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL); +} + void part_set_generic_name(const struct blk_desc *dev_desc, int part_num, char *name) { diff --git a/include/part.h b/include/part.h index 86117a7ce5..1a61518722 100644 --- a/include/part.h +++ b/include/part.h @@ -173,6 +173,21 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, struct blk_desc **dev_desc, disk_partition_t *info, int allow_whole_dev); +/** + * part_get_info_by_name_type() - Search for a partition by name + * for only specified partition type + * + * @param dev_desc - block device descriptor + * @param gpt_name - the specified table entry name + * @param info - returns the disk partition info + * @param part_type - only search in partitions of this type + * + * @return - the partition number on match (starting on 1), -1 on no match, + * otherwise error + */ +int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name, + disk_partition_t *info, int part_type); + /** * part_get_info_by_name() - Search for a partition by name * among all available registered partitions -- 2.14.1

3 6