Fwd: New Defects reported by Coverity Scan for Das U-Boot
Tom Rini
trini at konsulko.com
Mon Jan 8 18:45:06 CET 2024
Hey all,
Now that I've merged next I've re-run Coverity to get a start on issues
that've been added since last run. The report isn't complete because of
the number of issues, sadly, but if someone is interested in a specific
area contact me off-list and I can provide access to the dashboard.
For the hush related issues, this would be a good chance to work with
upstream and then backport the changes I suspect.
---------- Forwarded message ---------
From: <scan-admin at coverity.com>
Date: Mon, Jan 8, 2024 at 12:24 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To: <tom.rini at gmail.com>
Hi,
Please find the latest report on new defect(s) introduced to Das
U-Boot found with Coverity Scan.
41 new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 20 of 41 defect(s)
** CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5518 in parse_dollar()
________________________________________________________________________________________________________
*** CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5518 in parse_dollar()
5512 break;
5513 if (--cnt == 0)
5514 goto bad_dollar_syntax;
5515 if (len_single_ch != '#' &&
strchr(VAR_SUBST_OPS, ch))
5516 /* ${NN<op>...} is valid */
5517 goto eat_until_closing;
>>> CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
>>> Using variable "ch" as an index to array "_ctype".
5518 if (!isdigit(ch))
5519 goto bad_dollar_syntax;
5520 }
5521 } else
5522 while (1) {
5523 unsigned pos;
** CID 477216: (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
/drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
________________________________________________________________________________________________________
*** CID 477216: (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
3915
3916 /*
3917 * pages_per_block and blocks_per_lun may not be a
power-of-2 size
3918 * (don't ask me who thought of this...). MTD assumes that these
3919 * dimensions will be power-of-2, so just truncate the
remaining area.
3920 */
>>> CID 477216: (BAD_SHIFT)
>>> In expression "1 << generic_fls((__u32)(__le32)p->pages_per_block) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->pages_per_block) - 1", is -1.
3921 mtd->erasesize = 1 <<
(fls(le32_to_cpu(p->pages_per_block)) - 1);
3922 mtd->erasesize *= mtd->writesize;
3923
3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
3925
3926 /* See erasesize comment */
/drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
3921 mtd->erasesize = 1 <<
(fls(le32_to_cpu(p->pages_per_block)) - 1);
3922 mtd->erasesize *= mtd->writesize;
3923
3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
3925
3926 /* See erasesize comment */
>>> CID 477216: (BAD_SHIFT)
>>> In expression "1 << generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", is -1.
3927 chip->chipsize = 1 << (fls(le32_to_cpu(p->blocks_per_lun)) - 1);
3928 chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
3929 chip->bits_per_cell = p->bits_per_cell;
3930
3931 if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
3932 chip->options |= NAND_BUSWIDTH_16;
** CID 477215: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
________________________________________________________________________________________________________
*** CID 477215: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
4972 pr_warn("No ECC functions supplied;
hardware ECC not possible\n");
4973 BUG();
4974 }
4975 if (!ecc->read_page)
4976 ecc->read_page = nand_read_page_hwecc_oob_first;
4977
>>> CID 477215: Control flow issues (MISSING_BREAK)
>>> The case for value "NAND_ECC_HW" is not terminated by a "break" statement.
4978 case NAND_ECC_HW:
4979 /* Use standard hwecc read page function? */
4980 if (!ecc->read_page)
4981 ecc->read_page = nand_read_page_hwecc;
4982 if (!ecc->write_page)
4983 ecc->write_page = nand_write_page_hwecc;
** CID 477214: Integer handling issues (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
________________________________________________________________________________________________________
*** CID 477214: Integer handling issues (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
4391
4392 nand_decode_bbm_options(mtd, chip);
4393
4394 /* Calculate the address shift from the page size */
4395 chip->page_shift = ffs(mtd->writesize) - 1;
4396 /* Convert chipsize to number of pages per chip -1 */
>>> CID 477214: Integer handling issues (BAD_SHIFT)
>>> In expression "chip->chipsize >> chip->page_shift", shifting by a negative amount has undefined behavior. The shift amount, "chip->page_shift", is -1.
4397 chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
4398
4399 chip->bbt_erase_shift = chip->phys_erase_shift =
4400 ffs(mtd->erasesize) - 1;
4401 if (chip->chipsize & 0xffffffff)
4402 chip->chip_shift = ffs((unsigned)chip->chipsize) - 1;
** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in dm_test_nand()
________________________________________________________________________________________________________
*** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in dm_test_nand()
61 ops.ooblen = mtd->oobsize;
62 ut_assertok(mtd_read_oob(mtd, mtd->erasesize, &ops));
63 ut_asserteq(0, oob[mtd_to_nand(mtd)->badblockpos]);
64
65 /* Generate some data and write it */
66 for (i = 0; i < size / sizeof(int); i++)
>>> CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
67 gold[i] = rand();
68 ut_assertok(nand_write_skip_bad(mtd, off, &length, NULL, U64_MAX,
69 (void *)gold, 0));
70 ut_asserteq(size, length);
71
72 /* Verify */
** CID 477212: Incorrect expression (SIZEOF_MISMATCH)
/lib/smbios.c: 595 in write_smbios_table()
________________________________________________________________________________________________________
*** CID 477212: Incorrect expression (SIZEOF_MISMATCH)
/lib/smbios.c: 595 in write_smbios_table()
589 * sandbox's DRAM buffer.
590 */
591 table_addr = (ulong)map_sysmem(tables, 0);
592
593 /* now go back and write the SMBIOS3 header */
594 se = map_sysmem(start_addr, sizeof(struct smbios_entry));
>>> CID 477212: Incorrect expression (SIZEOF_MISMATCH)
>>> Passing argument "se" of type "struct smbios3_entry *" and argument "31UL" ("sizeof (struct smbios_entry)") to function "memset" is suspicious because a multiple of "sizeof (struct smbios3_entry) /*24*/" is expected.
595 memset(se, '\0', sizeof(struct smbios_entry));
596 memcpy(se->anchor, "_SM3_", 5);
597 se->length = sizeof(struct smbios3_entry);
598 se->major_ver = SMBIOS_MAJOR_VER;
599 se->minor_ver = SMBIOS_MINOR_VER;
600 se->doc_rev = 0;
** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
/drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
________________________________________________________________________________________________________
*** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
/drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
1127
1128 /* Prevent the bbt regions from erasing / writing */
1129 mark_bbt_region(mtd, td);
1130 if (md)
1131 mark_bbt_region(mtd, md);
1132
>>> CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
>>> Calling "vfree" frees "buf" using "vfree" but it should have been freed using "kfree". [Note: The source code implementation of the function has been overridden by a builtin model.]
1133 vfree(buf);
1134 return 0;
1135
1136 err:
1137 kfree(this->bbt);
1138 this->bbt = NULL;
** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
/drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
________________________________________________________________________________________________________
*** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
/drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
193 chip->tmp_dirty = true;
194 for (i = 0; i < chip->err_steps; i++) {
195 u32 bit_errors = chip->err_count;
196 unsigned int j = chip->err_step_bits + chip->ecc_bits;
197
198 while (bit_errors) {
>>> CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
199 unsigned int u = rand();
200 float quot = 1ULL << 32;
201
202 do {
203 quot *= j - bit_errors;
204 quot /= j;
** CID 477209: Memory - illegal accesses (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477209: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 4434 in reserved_word()
4428 str = old->as_string.data + len;
4429 if (str > old->as_string.data)
4430 str--; /* skip whitespace
after keyword */
4431 while (str > old->as_string.data &&
isalpha(str[-1]))
4432 str--;
4433 /* Ugh, we're done with this horrid hack */
>>> CID 477209: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "str" to "sandbox_strdup", which expects a null-terminated string.
4434 old->command->group_as_string = xstrdup(str);
4435 debug_printf_parse("pop, remembering as:'%s'\n",
4436 old->command->group_as_string);
4437 }
4438 # endif
4439 *ctx = *old; /* physical copy */
** CID 477208: Memory - illegal accesses (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477208: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 7660 in expand_variables()
7654 output.o_expflags = expflags;
7655
7656 n = 0;
7657 for (;;) {
7658 /* go to next list[n] */
7659 output.ended_in_ifs = 0;
>>> CID 477208: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "output.data" to "o_save_ptr", which expects a null-terminated string.
7660 n = o_save_ptr(&output, n);
7661
7662 if (!*argv)
7663 break;
7664
7665 /* expand argv[i] */
** CID 477207: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
________________________________________________________________________________________________________
*** CID 477207: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
4963 /*
4964 * Check ECC mode, default to software if
3byte/512byte hardware ECC is
4965 * selected and we have 256 byte pagesize fallback to
software ECC
4966 */
4967
4968 switch (ecc->mode) {
>>> CID 477207: Control flow issues (MISSING_BREAK)
>>> The case for value "NAND_ECC_HW_OOB_FIRST" is not terminated by a "break" statement.
4969 case NAND_ECC_HW_OOB_FIRST:
4970 /* Similar to NAND_ECC_HW, but a separate
read_page handle */
4971 if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
4972 pr_warn("No ECC functions supplied;
hardware ECC not possible\n");
4973 BUG();
4974 }
** CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5544 in parse_dollar()
________________________________________________________________________________________________________
*** CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5544 in parse_dollar()
5538 * So, we need to authorize # to appear inside
5539 * variable name and then expand this variable.
5540 * NOTE Having # in variable name is
not permitted in
5541 * upstream hush but expansion will be
done (even though
5542 * the result will be empty).
5543 */
>>> CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
>>> Using variable "ch" as an index to array "_ctype".
5544 if (!isalnum(ch) && ch != '_' && ch != '#') {
5545 #endif /* __U_BOOT__ */
5546 unsigned end_ch;
5547 #ifndef __U_BOOT__
5548 unsigned char last_ch;
5549 #endif /* !__U_BOOT__ */
** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 88 in mtd_dump_device_buf()
________________________________________________________________________________________________________
*** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 88 in mtd_dump_device_buf()
82 printf("\nDump %d data bytes from 0x%08llx:\n",
83 mtd->writesize, start_off + data_off);
84 mtd_dump_buf(&buf[data_off],
85 mtd->writesize, start_off + data_off);
86
87 if (woob) {
>>> CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "page * mtd->oobsize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
88 u64 oob_off = page * mtd->oobsize;
89
90 printf("Dump %d OOB bytes from page at
0x%08llx:\n",
91 mtd->oobsize, start_off + data_off);
92 mtd_dump_buf(&buf[len + oob_off],
93 mtd->oobsize, 0);
** CID 477204: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 10553 in run_list()
________________________________________________________________________________________________________
*** CID 477204: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 10553 in run_list()
10547 /* We cannot use xasprintf, so we emulate it. */
10548 char *full_var;
10549 char *var = pi->cmds[0].argv[0];
10550 char *val = *for_lcur++;
10551
10552 /* + 1 to take into account =. */
>>> CID 477204: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "val" to "strlen", which expects a null-terminated string. [Note: The source code implementation of the function has been overridden by a builtin model.]
10553 full_var = xmalloc(strlen(var) +
strlen(val) + 1);
10554 sprintf(full_var, "%s=%s", var, val);
10555
10556 set_local_var_modern(full_var, /*flag:*/ 0);
10557 #endif /* __U_BOOT__ */
10558 continue;
** CID 477203: (UNINIT)
/boot/bootm.c: 705 in bootm_load_os()
/boot/bootm.c: 713 in bootm_load_os()
________________________________________________________________________________________________________
*** CID 477203: (UNINIT)
/boot/bootm.c: 705 in bootm_load_os()
699 printf("Failed to prep arm64 kernel
(err=%d)\n", ret);
700 return BOOTM_ERR_RESET;
701 }
702
703 /* Handle BOOTM_STATE_LOADOS */
704 if (relocated_addr != load) {
>>> CID 477203: (UNINIT)
>>> Using uninitialized value "image_size".
705 printf("Moving Image from 0x%lx to
0x%lx, end=%lx\n",
706 load, relocated_addr,
707 relocated_addr + image_size);
708 memmove((void *)relocated_addr,
load_buf, image_size);
709 }
710
/boot/bootm.c: 713 in bootm_load_os()
707 relocated_addr + image_size);
708 memmove((void *)relocated_addr,
load_buf, image_size);
709 }
710
711 images->ep = relocated_addr;
712 images->os.start = relocated_addr;
>>> CID 477203: (UNINIT)
>>> Using uninitialized value "image_size".
713 images->os.end = relocated_addr + image_size;
714 }
715
716 lmb_reserve(&images->lmb, images->os.load, (load_end -
717 images->os.load));
718 return 0;
** CID 477202: Null pointer dereferences (FORWARD_NULL)
________________________________________________________________________________________________________
*** CID 477202: Null pointer dereferences (FORWARD_NULL)
/common/cli_hush_upstream.c: 4425 in reserved_word()
4419 * with "if " remaining in old->as_string!
4420 */
4421 {
4422 char *str;
4423 int len = old->as_string.length;
4424 /* Concatenate halves */
>>> CID 477202: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "ctx->as_string.data" to "o_addstr", which dereferences it.
4425 o_addstr(&old->as_string, ctx->as_string.data);
4426 o_free(&ctx->as_string);
4427 /* Find where leading keyword starts
in first half */
4428 str = old->as_string.data + len;
4429 if (str > old->as_string.data)
4430 str--; /* skip whitespace
after keyword */
** CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 80 in mtd_dump_device_buf()
________________________________________________________________________________________________________
*** CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 80 in mtd_dump_device_buf()
74 mtd->type == MTD_MLCNANDFLASH;
75 int npages = mtd_len_to_pages(mtd, len);
76 uint page;
77
78 if (has_pages) {
79 for (page = 0; page < npages; page++) {
>>> CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "page * mtd->writesize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
80 u64 data_off = page * mtd->writesize;
81
82 printf("\nDump %d data bytes from 0x%08llx:\n",
83 mtd->writesize, start_off + data_off);
84 mtd_dump_buf(&buf[data_off],
85 mtd->writesize, start_off + data_off);
** CID 477200: Security best practices violations (STRING_OVERFLOW)
/boot/bootm.c: 499 in bootm_find_images()
________________________________________________________________________________________________________
*** CID 477200: Security best practices violations (STRING_OVERFLOW)
/boot/bootm.c: 499 in bootm_find_images()
493 int ret;
494
495 if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
496 /* Look for an Android boot image */
497 buf = map_sysmem(images.os.start, 0);
498 if (buf && genimg_get_format(buf) ==
IMAGE_FORMAT_ANDROID) {
>>> CID 477200: Security best practices violations (STRING_OVERFLOW)
>>> You might overrun the 17-character fixed-size string "addr_str" by copying the return value of "simple_xtoa" without checking the length.
499 strcpy(addr_str, simple_xtoa(img_addr));
500 select = addr_str;
501 }
502 }
503
504 if (conf_ramdisk)
** CID 477199: (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477199: (STRING_NULL)
/common/cli_hush_upstream.c: 10315 in run_pipe()
10309 if (cmd_no < pi->num_cmds)
10310 close(pipefds.wr);
10311 /* Pass read (output) pipe end to next iteration */
10312 next_infd = pipefds.rd;
10313 #else /* __U_BOOT__ */
10314 /* Process the command */
>>> CID 477199: (STRING_NULL)
>>> Passing unterminated string "*command->argv" to "cmd_process", which expects a null-terminated string.
10315 rcode = cmd_process(G.do_repeat ? CMD_FLAG_REPEAT : 0,
10316 command->argc, command->argv,
10317 &(G.flag_repeat), NULL);
10318
10319 if (argv_expanded) {
10320 /*
/common/cli_hush_upstream.c: 9984 in run_pipe()
9978 }
9979 #endif
9980 debug_printf_env("set shell
var:'%s'->'%s'\n", *argv, p);
9981 #ifndef __U_BOOT__
9982 if (set_local_var0(p)) {
9983 #else /* __U_BOOT__ */
>>> CID 477199: (STRING_NULL)
>>> Passing unterminated string "p" to "set_local_var_modern", which expects a null-terminated string.
9984 if (set_local_var_modern(p,
/*flag:*/ 0)) {
9985 #endif
9986 /* assignment to
readonly var / putenv error? */
9987 rcode = 1;
9988 }
9989 i++;
** CID 477198: Control flow issues (DEADCODE)
/cmd/bootflow.c: 547 in do_bootflow_cmdline()
________________________________________________________________________________________________________
*** CID 477198: Control flow issues (DEADCODE)
/cmd/bootflow.c: 547 in do_bootflow_cmdline()
541 }
542
543 op = argv[1];
544 arg = argv[2];
545 if (*op == 's') {
546 if (argc < 3)
>>> CID 477198: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return CMD_RET_USAGE;".
547 return CMD_RET_USAGE;
548 val = argv[3] ?: (const char *)BOOTFLOWCL_EMPTY;
549 }
550
551 switch (*op) {
552 case 'c': /* clear */
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240108/bde985a1/attachment.sig>
More information about the U-Boot
mailing list