* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-01-19 8:47 ` Fwd: " Heinrich Schuchardt
@ 2024-01-22 6:44 ` Masahisa Kojima
0 siblings, 0 replies; 19+ messages in thread
From: Masahisa Kojima @ 2024-01-22 6:44 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: U-Boot Mailing List, Ilias Apalodimas
Hi Heinrich,
On Fri, 19 Jan 2024 at 17:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> ________________________________________________________________________________________________________
> *** CID 478333: Error handling issues (CHECKED_RETURN)
> /lib/efi_loader/efi_firmware.c: 413 in efi_firmware_set_fmp_state_var()
> 407 /*
> 408 * GetVariable may fail, EFI_NOT_FOUND is returned if FmpState
> 409 * variable has not been set yet.
> 410 * Ignore the error here since the correct FmpState variable
> 411 * is set later.
> 412 */
> >>> CID 478333: Error handling issues (CHECKED_RETURN)
> >>> Calling "efi_get_variable_int" without checking return value (as is done elsewhere 29 out of 33 times).
> 413 efi_get_variable_int(varname, image_type_id, NULL, &size,
> var_state,
> 414 NULL);
> 415 416 /*
> 417 * Only the fw_version is set here.
> 418 * lowest_supported_version in FmpState variable is ignored since
>
> There are a lot of different return values that may occur when calling
> efi_get_variable_int, e.g.
>
> * EFI_BUFFER_TOO_SMALL
> * EFI_DEVICE_ERROR
>
> Should we overwrite the variable in these cases with NUL values except
> for var_state[update_bank].fw_version?
The var_state buffer is allocated by calloc(), and efi_get_variable_int()
will not update the buffer in case of error.
But it is better to set NUL values to var_state, I will send a fix.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-04-22 21:48 Fwd: " Tom Rini
@ 2024-04-23 6:19 ` Ilias Apalodimas
0 siblings, 0 replies; 19+ messages in thread
From: Ilias Apalodimas @ 2024-04-23 6:19 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, Charles Hardin
Hi Tom,
Thanks! Already sent a fix for UEFI
On Tue, 23 Apr 2024 at 00:48, Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Apr 22, 2024 at 3:23 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 7 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 2 of 2 defect(s)
>
>
> ** CID 492766: Control flow issues (DEADCODE)
> /lib/efi_loader/efi_var_mem.c: 236 in efi_var_mem_init()
>
>
> ________________________________________________________________________________________________________
> *** CID 492766: Control flow issues (DEADCODE)
> /lib/efi_loader/efi_var_mem.c: 236 in efi_var_mem_init()
> 230 memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
> 231 efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
> 232 efi_var_buf->length = (uintptr_t)efi_var_buf->var -
> 233 (uintptr_t)efi_var_buf;
> 234
> 235 if (ret != EFI_SUCCESS)
> >>> CID 492766: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "return ret;".
> 236 return ret;
> 237 ret =
> efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
> 238
> efi_var_mem_notify_virtual_address_map, NULL,
> 239 NULL, &event);
> 240 if (ret != EFI_SUCCESS)
> 241 return ret;
>
> ** CID 492765: Uninitialized variables (UNINIT)
>
>
> ________________________________________________________________________________________________________
> *** CID 492765: Uninitialized variables (UNINIT)
> /net/bootp.c: 888 in dhcp_process_options()
> 882 net_root_path[size] = 0;
> 883 break;
> 884 case 28: /* Ignore Broadcast Address Option */
> 885 break;
> 886 case 40: /* NIS Domain name */
> 887 if (net_nis_domain[0] == 0) {
> >>> CID 492765: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "size" when calling "truncate_sz".
> 888 size = truncate_sz("NIS Domain Name",
> 889 sizeof(net_nis_domain), size);
> 890 memcpy(&net_nis_domain, popt + 2, size);
> 891 net_nis_domain[size] = 0;
> 892 }
> 893 break;
>
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-10-07 17:15 Fwd: " Tom Rini
@ 2024-10-07 18:17 ` Richard Weinberger
2024-10-07 20:01 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Richard Weinberger @ 2024-10-07 18:17 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Simon Glass, Joao Marcos Costa, Thomas Petazzoni,
Jerome Forissier, Sughosh Ganu, Caleb Connolly
----- Ursprüngliche Mail -----
> Von: "Tom Rini" <trini@konsulko.com>
> An: "u-boot" <u-boot@lists.denx.de>
> CC: "Simon Glass" <sjg@chromium.org>, "Joao Marcos Costa" <jmcosta944@gmail.com>, "Thomas Petazzoni"
> <thomas.petazzoni@bootlin.com>, "Jerome Forissier" <jerome.forissier@linaro.org>, "Sughosh Ganu"
> <sughosh.ganu@linaro.org>, "Caleb Connolly" <caleb.connolly@linaro.org>, "richard" <richard@nod.at>
> Gesendet: Montag, 7. Oktober 2024 19:15:05
> Betreff: Fwd: New Defects reported by Coverity Scan for Das U-Boot
> Now that I've merged next to master, there's a number of issues to
> address.
AFAICT, all issues in fs/{ext4, squashfs} are not new.
Looks more like Coverity re-scanned these files because they have been touched
by my fixes.
Later this week I'll have more time to triage these issues and post fixes.
Thanks,
//richard
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-10-07 18:17 ` Richard Weinberger
@ 2024-10-07 20:01 ` Tom Rini
0 siblings, 0 replies; 19+ messages in thread
From: Tom Rini @ 2024-10-07 20:01 UTC (permalink / raw)
To: Richard Weinberger
Cc: u-boot, Simon Glass, Joao Marcos Costa, Thomas Petazzoni,
Jerome Forissier, Sughosh Ganu, Caleb Connolly
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
On Mon, Oct 07, 2024 at 08:17:44PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Tom Rini" <trini@konsulko.com>
> > An: "u-boot" <u-boot@lists.denx.de>
> > CC: "Simon Glass" <sjg@chromium.org>, "Joao Marcos Costa" <jmcosta944@gmail.com>, "Thomas Petazzoni"
> > <thomas.petazzoni@bootlin.com>, "Jerome Forissier" <jerome.forissier@linaro.org>, "Sughosh Ganu"
> > <sughosh.ganu@linaro.org>, "Caleb Connolly" <caleb.connolly@linaro.org>, "richard" <richard@nod.at>
> > Gesendet: Montag, 7. Oktober 2024 19:15:05
> > Betreff: Fwd: New Defects reported by Coverity Scan for Das U-Boot
>
> > Now that I've merged next to master, there's a number of issues to
> > address.
>
> AFAICT, all issues in fs/{ext4, squashfs} are not new.
> Looks more like Coverity re-scanned these files because they have been touched
> by my fixes.
Quite likely, yeah.
> Later this week I'll have more time to triage these issues and post fixes.
Thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-10-16 3:47 Fwd: " Tom Rini
@ 2024-10-16 6:12 ` Ilias Apalodimas
2024-10-16 8:20 ` Abbarapu, Venkatesh
2024-10-16 15:23 ` Raymond Mao
1 sibling, 1 reply; 19+ messages in thread
From: Ilias Apalodimas @ 2024-10-16 6:12 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
Venkatesh Yadav Abbarapu, Pratyush Yadav, Ashok Reddy Soma,
Joakim Tjernlund, Raymond Mao
HI Tom,
We'll have a look on the mbedTLS reports later today
Thanks
/Ilias
On Wed, 16 Oct 2024 at 06:47, Tom Rini <trini@konsulko.com> wrote:
>
> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 20 of 22 defect(s)
>
>
> ** CID 510813: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
>
>
> ________________________________________________________________________________________________________
> *** CID 510813: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> 1646 goto read_err;
> 1647 }
> 1648 if (ret < 0)
> 1649 goto read_err;
> 1650
> 1651 if (is_ofst_odd == true) {
> >>> CID 510813: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "memmove(buf, buf + 1, len -...".
> 1652 memmove(buf, (buf + 1), (len - 1));
> 1653 *retlen += (ret - 1);
> 1654 buf += ret - 1;
> 1655 is_ofst_odd = false;
> 1656 } else {
> 1657 *retlen += ret;
>
> ** CID 510812: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510812: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> 3567 /*
> 3568 * In parallel-memories the erase operation is
> 3569 * performed on both the flashes simultaneously
> 3570 * so, double the erasesize.
> 3571 */
> 3572 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510812: (DEADCODE)
> >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3573 mtd->erasesize = 4096 * 2;
> 3574 else
> 3575 mtd->erasesize = 4096;
> 3576 } else if (info->flags & SECT_4K_PMC) {
> 3577 nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> 3578 /*
> /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> 3578 /*
> 3579 * In parallel-memories the erase operation is
> 3580 * performed on both the flashes simultaneously
> 3581 * so, double the erasesize.
> 3582 */
> 3583 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510812: (DEADCODE)
> >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3584 mtd->erasesize = 4096 * 2;
> 3585 else
> 3586 mtd->erasesize = 4096;
> 3587 } else
> 3588 #endif
> 3589 {
> /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> 3604 /*
> 3605 * In parallel-memories the erase operation is
> 3606 * performed on both the flashes simultaneously
> 3607 * so, double the erasesize.
> 3608 */
> 3609 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510812: (DEADCODE)
> >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3610 mtd->erasesize = 4096 * 2;
> 3611 else
> 3612 mtd->erasesize = 4096;
> 3613 }
> 3614
> 3615 return 0;
> /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> 3591 /*
> 3592 * In parallel-memories the erase operation is
> 3593 * performed on both the flashes simultaneously
> 3594 * so, double the erasesize.
> 3595 */
> 3596 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510812: (DEADCODE)
> >>> Execution cannot reach this statement: "mtd->erasesize = info->sect...".
> 3597 mtd->erasesize = info->sector_size * 2;
> 3598 else
> 3599 mtd->erasesize = info->sector_size;
> 3600 }
> 3601
> 3602 if ((JEDEC_MFR(info) == SNOR_MFR_SST) && info->flags &
> SECT_4K) {
>
> ** CID 510811: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510811: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> 1128 addr_known = false;
> 1129 ret = -EINTR;
> 1130 goto erase_err;
> 1131 }
> 1132 offset = addr;
> 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510811: (DEADCODE)
> >>> Execution cannot reach this statement: "offset /= 2U;".
> 1134 offset /= 2;
> 1135
> 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> 1137 if (offset >= (mtd->size / 2)) {
> 1138 offset = offset - (mtd->size / 2);
> 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> 1131 }
> 1132 offset = addr;
> 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> 1134 offset /= 2;
> 1135
> 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> >>> CID 510811: (DEADCODE)
> >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 1137 if (offset >= (mtd->size / 2)) {
> 1138 offset = offset - (mtd->size / 2);
> 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> 1140 } else {
> 1141 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1142 }
>
> ** CID 510810: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
>
>
> ________________________________________________________________________________________________________
> *** CID 510810: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> 1550 {
> 1551 int tmp;
> 1552 u8 id[SPI_NOR_MAX_ID_LEN];
> 1553 const struct flash_info *info;
> 1554
> 1555 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510810: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "nor->spi->flags |= 0x100;".
> 1556 nor->spi->flags |= SPI_XFER_LOWER;
> 1557
> 1558 tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> SPI_NOR_MAX_ID_LEN);
> 1559 if (tmp < 0) {
> 1560 dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> 1561 return ERR_PTR(tmp);
>
> ** CID 510809: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
>
>
> ________________________________________________________________________________________________________
> *** CID 510809: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> 379 signed_info);
> 380 if (ret)
> 381 goto out_err_sinfo;
> 382
> 383 no_authattrs:
> 384 *sinfo = signed_info;
> >>> CID 510809: Resource leaks (RESOURCE_LEAK)
> >>> Variable "mctx" going out of scope leaks the storage it points to.
> 385 return 0;
> 386
> 387 out_err_sinfo:
> 388 pkcs7_free_sinfo_mbedtls_ctx(mctx);
> 389 out_no_mctx:
> 390 public_key_signature_free(s);
>
> ** CID 510808: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
>
>
> ________________________________________________________________________________________________________
> *** CID 510808: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> 718 static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> 719 const struct flash_info *info)
> 720 {
> 721 bool shift = 0;
> 722
> 723 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510808: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "shift = true;".
> 724 shift = 1;
> 725
> 726 /* Do some manufacturer fixups first */
> 727 switch (JEDEC_MFR(info)) {
> 728 case SNOR_MFR_SPANSION:
> 729 /* No small sector erase for 4-byte command set */
>
> ** CID 510807: Control flow issues (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
>
>
> ________________________________________________________________________________________________________
> *** CID 510807: Control flow issues (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> 2744 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> 2745 nonzero_groups++;
> 2746 if (*p == '\0') {
> 2747 break;
> 2748 } else if (*p == '.') {
> 2749 /* Don't accept IPv4 too early or late */
> >>> CID 510807: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "zero_group_start == -1" inside this statement: "if ((nonzero_groups == 0 &&...".
> 2750 if ((nonzero_groups == 0 && zero_group_start == -1) ||
> 2751 nonzero_groups >= 7) {
> 2752 break;
> 2753 }
> 2754
> 2755 /* Walk back to prior ':', then parse as IPv4-mapped */
>
> ** CID 510806: Control flow issues (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
>
>
> ________________________________________________________________________________________________________
> *** CID 510806: Control flow issues (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> 203 return -EINVAL;
> 204 }
> 205
> 206 p += seq_len;
> 207 }
> 208
> >>> CID 510806: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "ret != -96" inside this statement: "if (ret && ret != -96)
> re...".
> 209 if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> 210 return ret;
> 211
> 212 msg->have_authattrs = true;
> 213
> 214 /*
>
> ** CID 510805: Memory - illegal accesses (OVERRUN)
> /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
>
>
> ________________________________________________________________________________________________________
> *** CID 510805: Memory - illegal accesses (OVERRUN)
> /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> 672 (*prop)->num_bits = (rsa_key.n_sz - i) * 8;
> 673 (*prop)->modulus = malloc(rsa_key.n_sz - i);
> 674 if (!(*prop)->modulus) {
> 675 ret = -ENOMEM;
> 676 goto out;
> 677 }
> >>> CID 510805: Memory - illegal accesses (OVERRUN)
> >>> Overrunning dynamic array "rsa_key.n" at offset corresponding to index variable "i".
> 678 memcpy((void *)(*prop)->modulus, &rsa_key.n[i],
> rsa_key.n_sz - i);
> 679
> 680 n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> 681 rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits
> * 2) >> 5));
> 682 rrtmp = calloc(sizeof(uint32_t), 2 +
> (((*prop)->num_bits * 2) >> 5));
> 683 if (!n || !rr || !rrtmp) {
>
> ** CID 510804: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
>
>
> ________________________________________________________________________________________________________
> *** CID 510804: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> 1550 {
> 1551 int tmp;
> 1552 u8 id[SPI_NOR_MAX_ID_LEN];
> 1553 const struct flash_info *info;
> 1554
> 1555 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510804: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> >>> In "nor->spi->flags |= 256 /* 1 << 8 */", wider "256 /* 1 << 8 */" has high-order bits (0x100) that don't affect the narrower left-hand side.
> 1556 nor->spi->flags |= SPI_XFER_LOWER;
> 1557
> 1558 tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> SPI_NOR_MAX_ID_LEN);
> 1559 if (tmp < 0) {
> 1560 dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> 1561 return ERR_PTR(tmp);
>
> ** CID 510803: Code maintainability issues (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510803: Code maintainability issues (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> 1132 offset = addr;
> 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> 1134 offset /= 2;
> 1135
> 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> 1137 if (offset >= (mtd->size / 2)) {
> >>> CID 510803: Code maintainability issues (UNUSED_VALUE)
> >>> Assigning value from "offset - mtd->size / 2ULL" to "offset" here, but that stored value is overwritten before it can be used.
> 1138 offset = offset - (mtd->size / 2);
> 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> 1140 } else {
> 1141 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1142 }
> 1143 }
>
> ** CID 510802: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
>
>
> ________________________________________________________________________________________________________
> *** CID 510802: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> 498 * discard the second byte.
> 499 */
> 500 if (spi_nor_protocol_is_dtr(nor->reg_proto))
> 501 op.data.nbytes = 2;
> 502
> 503 if (nor->flags & SNOR_F_HAS_PARALLEL) {
> >>> CID 510802: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "op.data.nbytes = 2U;".
> 504 op.data.nbytes = 2;
> 505 ret = spi_nor_read_write_reg(nor, &op, &val[0]);
> 506 if (ret < 0) {
> 507 pr_debug("error %d reading SR\n", (int)ret);
> 508 return ret;
> 509 }
>
> ** CID 510801: Null pointer dereferences (FORWARD_NULL)
>
>
> ________________________________________________________________________________________________________
> *** CID 510801: Null pointer dereferences (FORWARD_NULL)
> /lib/ecdsa/ecdsa-libcrypto.c: 365 in ecdsa_add_verify_data()
> 359 struct signer ctx;
> 360 int ret;
> 361
> 362 fdt_key_name = info->keyname ? info->keyname : "default-key";
> 363 ret = prepare_ctx(&ctx, info);
> 364 if (ret >= 0) {
> >>> CID 510801: Null pointer dereferences (FORWARD_NULL)
> >>> Passing "info" to "do_add", which dereferences null "info->keyname".
> 365 ret = do_add(&ctx, fdt, fdt_key_name, info);
> 366 if (ret < 0)
> 367 ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> 368 }
> 369
> 370 free_ctx(&ctx);
> 371 return ret;
>
> ** CID 510800: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
>
>
> ________________________________________________________________________________________________________
> *** CID 510800: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> 1614 } else {
> 1615 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1616 }
> 1617 }
> 1618
> 1619 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510800: (DEADCODE)
> >>> Execution cannot reach this statement: "offset /= 2LL;".
> 1620 offset /= 2;
> 1621
> 1622 if (nor->addr_width == 3) {
> 1623 #ifdef CONFIG_SPI_FLASH_BAR
> 1624 ret = write_bar(nor, offset);
> 1625 if (ret < 0)
> /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> 1584 u32 rem_bank_len = 0;
> 1585 u8 bank;
> 1586 bool is_ofst_odd = false;
> 1587
> 1588 dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> 1589
> >>> CID 510800: (DEADCODE)
> >>> Execution cannot reach the expression "offset & 1LL" inside this statement: "if (nor->flags & SNOR_F_HAS...".
> 1590 if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) {
> 1591 /* We can hit this case when we use file system
> like ubifs */
> 1592 from--;
> 1593 len++;
> 1594 is_ofst_odd = true;
> 1595 }
> /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> 1605 rem_bank_len = (SZ_16M * (bank
> + 1)) - from;
> 1606 }
> 1607 }
> 1608 offset = from;
> 1609
> 1610 if (nor->flags & SNOR_F_HAS_STACKED) {
> >>> CID 510800: (DEADCODE)
> >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 1611 if (offset >= (mtd->size / 2)) {
> 1612 offset = offset - (mtd->size / 2);
> 1613 nor->spi->flags |= SPI_XFER_U_PAGE;
> 1614 } else {
> 1615 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1616 }
> /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> 1594 is_ofst_odd = true;
> 1595 }
> 1596
> 1597 while (len) {
> 1598 if (nor->addr_width == 3) {
> 1599 if (nor->flags & SNOR_F_HAS_PARALLEL) {
> >>> CID 510800: (DEADCODE)
> >>> Execution cannot reach this statement: "bank = (u32)from / 33554432U;".
> 1600 bank = (u32)from / (SZ_16M << 0x01);
> 1601 rem_bank_len = ((SZ_16M << 0x01) *
> 1602 (bank + 1)) - from;
> 1603 } else {
> 1604 bank = (u32)from / SZ_16M;
> 1605 rem_bank_len = (SZ_16M * (bank
> + 1)) - from;
>
> ** CID 510799: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
>
>
> ________________________________________________________________________________________________________
> *** CID 510799: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> 1965 return 0;
> 1966
> 1967 /*
> 1968 * Cannot write to odd offset in parallel mode,
> 1969 * so write 2 bytes first
> 1970 */
> >>> CID 510799: (DEADCODE)
> >>> Execution cannot reach the expression "to & 1LL" inside this statement: "if (nor->flags & SNOR_F_HAS...".
> 1971 if ((nor->flags & SNOR_F_HAS_PARALLEL) && (to & 1)) {
> 1972 u8 two[2] = {0xff, buf[0]};
> 1973 size_t local_retlen;
> 1974
> 1975 ret = spi_nor_write(mtd, to & ~1, 2,
> &local_retlen, two);
> 1976 if (ret < 0)
> /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> 2001 }
> 2002 offset = (to + i);
> 2003 if (nor->flags & SNOR_F_HAS_PARALLEL)
> 2004 offset /= 2;
> 2005
> 2006 if (nor->flags & SNOR_F_HAS_STACKED) {
> >>> CID 510799: (DEADCODE)
> >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 2007 if (offset >= (mtd->size / 2)) {
> 2008 offset = offset - (mtd->size / 2);
> 2009 nor->spi->flags |= SPI_XFER_U_PAGE;
> 2010 } else {
> 2011 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 2012 }
> /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> 1998 u64 aux = addr;
> 1999
> 2000 page_offset = do_div(aux, nor->page_size);
> 2001 }
> 2002 offset = (to + i);
> 2003 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510799: (DEADCODE)
> >>> Execution cannot reach this statement: "offset /= 2U;".
> 2004 offset /= 2;
> 2005
> 2006 if (nor->flags & SNOR_F_HAS_STACKED) {
> 2007 if (offset >= (mtd->size / 2)) {
> 2008 offset = offset - (mtd->size / 2);
> 2009 nor->spi->flags |= SPI_XFER_U_PAGE;
>
> ** CID 510798: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
>
>
> ________________________________________________________________________________________________________
> *** CID 510798: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> 214 }
> 215
> 216 ret = hash_calculate(s->hash_algo, ®ion, 1, s->digest);
> 217 if (!ret)
> 218 *sig = s;
> 219
> >>> CID 510798: Resource leaks (RESOURCE_LEAK)
> >>> Variable "s" going out of scope leaks the storage it points to.
> 220 return ret;
> 221
> 222 error_sig:
> 223 public_key_signature_free(s);
> 224 return ret;
> 225 }
>
> ** CID 510797: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
>
>
> ________________________________________________________________________________________________________
> *** CID 510797: (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> 4622 /* Send all the required SPI flash commands to
> initialize device */
> 4623 ret = spi_nor_init(nor);
> 4624 if (ret)
> 4625 return ret;
> 4626
> 4627 if (nor->flags & SNOR_F_HAS_STACKED) {
> >>> CID 510797: (DEADCODE)
> >>> Execution cannot reach this statement: "nor->spi->flags |= 0x10UL;".
> 4628 nor->spi->flags |= SPI_XFER_U_PAGE;
> 4629 ret = spi_nor_init(nor);
> 4630 if (ret)
> 4631 return ret;
> 4632 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 4633 }
> /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> 4592 nor->addr_width = info->addr_width;
> 4593 } else {
> 4594 nor->addr_width = 3;
> 4595 }
> 4596
> 4597 if (nor->flags & (SNOR_F_HAS_PARALLEL | SNOR_F_HAS_STACKED))
> >>> CID 510797: (DEADCODE)
> >>> Execution cannot reach this statement: "shift = true;".
> 4598 shift = 1;
> 4599 if (nor->addr_width == 3 && (mtd->size >> shift) > SZ_16M) {
> 4600 #ifndef CONFIG_SPI_FLASH_BAR
> 4601 /* enable 4-byte addressing if the device
> exceeds 16MiB */
> 4602 nor->addr_width = 4;
> 4603 if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>
> ** CID 510796: Insecure data handling (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 510796: Insecure data handling (TAINTED_SCALAR)
> /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> 1310 }
> 1311
> 1312 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> ctx->len - 1, f_rng, p_rng));
> 1313
> 1314 /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> inv_mod. */
> 1315 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> - 1, f_rng, p_rng));
> >>> CID 510796: Insecure data handling (TAINTED_SCALAR)
> >>> Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi", which uses it as an offset.
> 1316 MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf, &R));
> 1317 MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> &ctx->Vi, &ctx->N));
> 1318
> 1319 /* At this point, Vi is invertible mod N if and only
> if both Vf and R
> 1320 * are invertible mod N. If one of them isn't, we
> don't need to know
> 1321 * which one, we just loop and choose new values for
> both of them.
>
> ** CID 510795: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
>
>
> ________________________________________________________________________________________________________
> *** CID 510795: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> 4265
> 4266 static int spi_nor_init(struct spi_nor *nor)
> 4267 {
> 4268 int err;
> 4269
> 4270 if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>> CID 510795: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "nor->spi->flags |= 3UL;".
> 4271 nor->spi->flags |= SPI_NOR_ENABLE_MULTI_CS;
> 4272
> 4273 err = spi_nor_octal_dtr_enable(nor);
> 4274 if (err) {
> 4275 dev_dbg(nor->dev, "Octal DTR mode not supported\n");
> 4276 return err;
>
> ** CID 510794: Control flow issues (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
>
>
> ________________________________________________________________________________________________________
> *** CID 510794: Control flow issues (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> 72 do {
> 73 name_str = kzalloc(len, GFP_KERNEL);
> 74 if (!name_str)
> 75 return NULL;
> 76
> 77 wb = mbedtls_x509_dn_gets(name_str, len, name);
> >>> CID 510794: Control flow issues (NO_EFFECT)
> >>> This less-than-zero comparison of an unsigned value is never true. "wb < 0UL".
> 78 if (wb < 0) {
> 79 pr_err("Get DN string failed, ret:-0x%04x\n",
> 80 (unsigned int)-wb);
> 81 kfree(name_str);
> 82 len = len * 2; /* Try with a bigger buffer */
> 83 }
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: New Defects reported by Coverity Scan for Das U-Boot
2024-10-16 6:12 ` Ilias Apalodimas
@ 2024-10-16 8:20 ` Abbarapu, Venkatesh
0 siblings, 0 replies; 19+ messages in thread
From: Abbarapu, Venkatesh @ 2024-10-16 8:20 UTC (permalink / raw)
To: Ilias Apalodimas, Tom Rini
Cc: u-boot@lists.denx.de, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
Pratyush Yadav, Ashok Reddy Soma, Joakim Tjernlund, Raymond Mao
Hi Tom,
I will try to check for the spi nor core issues mentioned below.
Thanks
Venkatesh
> -----Original Message-----
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Sent: Wednesday, October 16, 2024 11:43 AM
> To: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de; Vignesh R <vigneshr@ti.com>; Takahiro Kuwano
> <Takahiro.Kuwano@infineon.com>; Tudor Ambarus <tudor.ambarus@linaro.org>;
> Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Pratyush Yadav
> <p.yadav@ti.com>; Ashok Reddy Soma <ashok.reddy.soma@amd.com>; Joakim
> Tjernlund <joakim.tjernlund@infinera.com>; Raymond Mao
> <raymond.mao@linaro.org>
> Subject: Re: New Defects reported by Coverity Scan for Das U-Boot
>
> HI Tom,
>
> We'll have a look on the mbedTLS reports later today
>
> Thanks
> /Ilias
>
> On Wed, 16 Oct 2024 at 06:47, Tom Rini <trini@konsulko.com> wrote:
> >
> > Hey all, here's the latest report.
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Tue, Oct 15, 2024 at 5:06 PM
> > Subject: New Defects reported by Coverity Scan for Das U-Boot
> > To: <tom.rini@gmail.com>
> >
> >
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to Das
> > U-Boot found with Coverity Scan.
> >
> > 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan Showing 20 of 22 defect(s)
> >
> >
> > ** CID 510813: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510813: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> > 1646 goto read_err;
> > 1647 }
> > 1648 if (ret < 0)
> > 1649 goto read_err;
> > 1650
> > 1651 if (is_ofst_odd == true) {
> > >>> CID 510813: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "memmove(buf, buf + 1, len -...".
> > 1652 memmove(buf, (buf + 1), (len - 1));
> > 1653 *retlen += (ret - 1);
> > 1654 buf += ret - 1;
> > 1655 is_ofst_odd = false;
> > 1656 } else {
> > 1657 *retlen += ret;
> >
> > ** CID 510812: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510812: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> > 3567 /*
> > 3568 * In parallel-memories the erase operation is
> > 3569 * performed on both the flashes simultaneously
> > 3570 * so, double the erasesize.
> > 3571 */
> > 3572 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510812: (DEADCODE)
> > >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3573 mtd->erasesize = 4096 * 2;
> > 3574 else
> > 3575 mtd->erasesize = 4096;
> > 3576 } else if (info->flags & SECT_4K_PMC) {
> > 3577 nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> > 3578 /*
> > /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> > 3578 /*
> > 3579 * In parallel-memories the erase operation is
> > 3580 * performed on both the flashes simultaneously
> > 3581 * so, double the erasesize.
> > 3582 */
> > 3583 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510812: (DEADCODE)
> > >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3584 mtd->erasesize = 4096 * 2;
> > 3585 else
> > 3586 mtd->erasesize = 4096;
> > 3587 } else
> > 3588 #endif
> > 3589 {
> > /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> > 3604 /*
> > 3605 * In parallel-memories the erase operation is
> > 3606 * performed on both the flashes simultaneously
> > 3607 * so, double the erasesize.
> > 3608 */
> > 3609 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510812: (DEADCODE)
> > >>> Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3610 mtd->erasesize = 4096 * 2;
> > 3611 else
> > 3612 mtd->erasesize = 4096;
> > 3613 }
> > 3614
> > 3615 return 0;
> > /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> > 3591 /*
> > 3592 * In parallel-memories the erase operation is
> > 3593 * performed on both the flashes simultaneously
> > 3594 * so, double the erasesize.
> > 3595 */
> > 3596 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510812: (DEADCODE)
> > >>> Execution cannot reach this statement: "mtd->erasesize = info->sect...".
> > 3597 mtd->erasesize = info->sector_size * 2;
> > 3598 else
> > 3599 mtd->erasesize = info->sector_size;
> > 3600 }
> > 3601
> > 3602 if ((JEDEC_MFR(info) == SNOR_MFR_SST) && info->flags &
> > SECT_4K) {
> >
> > ** CID 510811: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510811: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> > 1128 addr_known = false;
> > 1129 ret = -EINTR;
> > 1130 goto erase_err;
> > 1131 }
> > 1132 offset = addr;
> > 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510811: (DEADCODE)
> > >>> Execution cannot reach this statement: "offset /= 2U;".
> > 1134 offset /= 2;
> > 1135
> > 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> > 1137 if (offset >= (mtd->size / 2)) {
> > 1138 offset = offset - (mtd->size / 2);
> > 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> > /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> > 1131 }
> > 1132 offset = addr;
> > 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 1134 offset /= 2;
> > 1135
> > 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>> CID 510811: (DEADCODE)
> > >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 1137 if (offset >= (mtd->size / 2)) {
> > 1138 offset = offset - (mtd->size / 2);
> > 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1140 } else {
> > 1141 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1142 }
> >
> > ** CID 510810: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510810: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> > 1550 {
> > 1551 int tmp;
> > 1552 u8 id[SPI_NOR_MAX_ID_LEN];
> > 1553 const struct flash_info *info;
> > 1554
> > 1555 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510810: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "nor->spi->flags |= 0x100;".
> > 1556 nor->spi->flags |= SPI_XFER_LOWER;
> > 1557
> > 1558 tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> > SPI_NOR_MAX_ID_LEN);
> > 1559 if (tmp < 0) {
> > 1560 dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> > 1561 return ERR_PTR(tmp);
> >
> > ** CID 510809: Resource leaks (RESOURCE_LEAK)
> > /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510809: Resource leaks (RESOURCE_LEAK)
> > /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> > 379 signed_info);
> > 380 if (ret)
> > 381 goto out_err_sinfo;
> > 382
> > 383 no_authattrs:
> > 384 *sinfo = signed_info;
> > >>> CID 510809: Resource leaks (RESOURCE_LEAK)
> > >>> Variable "mctx" going out of scope leaks the storage it points to.
> > 385 return 0;
> > 386
> > 387 out_err_sinfo:
> > 388 pkcs7_free_sinfo_mbedtls_ctx(mctx);
> > 389 out_no_mctx:
> > 390 public_key_signature_free(s);
> >
> > ** CID 510808: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510808: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> > 718 static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> > 719 const struct flash_info *info)
> > 720 {
> > 721 bool shift = 0;
> > 722
> > 723 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510808: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "shift = true;".
> > 724 shift = 1;
> > 725
> > 726 /* Do some manufacturer fixups first */
> > 727 switch (JEDEC_MFR(info)) {
> > 728 case SNOR_MFR_SPANSION:
> > 729 /* No small sector erase for 4-byte command set */
> >
> > ** CID 510807: Control flow issues (DEADCODE)
> > /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510807: Control flow issues (DEADCODE)
> > /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> > 2744 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> > 2745 nonzero_groups++;
> > 2746 if (*p == '\0') {
> > 2747 break;
> > 2748 } else if (*p == '.') {
> > 2749 /* Don't accept IPv4 too early or late */
> > >>> CID 510807: Control flow issues (DEADCODE)
> > >>> Execution cannot reach the expression "zero_group_start == -1" inside this
> statement: "if ((nonzero_groups == 0 &&...".
> > 2750 if ((nonzero_groups == 0 && zero_group_start == -1) ||
> > 2751 nonzero_groups >= 7) {
> > 2752 break;
> > 2753 }
> > 2754
> > 2755 /* Walk back to prior ':', then parse as IPv4-mapped */
> >
> > ** CID 510806: Control flow issues (DEADCODE)
> > /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510806: Control flow issues (DEADCODE)
> > /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> > 203 return -EINVAL;
> > 204 }
> > 205
> > 206 p += seq_len;
> > 207 }
> > 208
> > >>> CID 510806: Control flow issues (DEADCODE)
> > >>> Execution cannot reach the expression "ret != -96" inside this statement: "if
> (ret && ret != -96)
> > re...".
> > 209 if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> > 210 return ret;
> > 211
> > 212 msg->have_authattrs = true;
> > 213
> > 214 /*
> >
> > ** CID 510805: Memory - illegal accesses (OVERRUN)
> > /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510805: Memory - illegal accesses (OVERRUN)
> > /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> > 672 (*prop)->num_bits = (rsa_key.n_sz - i) * 8;
> > 673 (*prop)->modulus = malloc(rsa_key.n_sz - i);
> > 674 if (!(*prop)->modulus) {
> > 675 ret = -ENOMEM;
> > 676 goto out;
> > 677 }
> > >>> CID 510805: Memory - illegal accesses (OVERRUN)
> > >>> Overrunning dynamic array "rsa_key.n" at offset corresponding to index
> variable "i".
> > 678 memcpy((void *)(*prop)->modulus, &rsa_key.n[i],
> > rsa_key.n_sz - i);
> > 679
> > 680 n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> > 681 rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits
> > * 2) >> 5));
> > 682 rrtmp = calloc(sizeof(uint32_t), 2 +
> > (((*prop)->num_bits * 2) >> 5));
> > 683 if (!n || !rr || !rrtmp) {
> >
> > ** CID 510804: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510804: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> > 1550 {
> > 1551 int tmp;
> > 1552 u8 id[SPI_NOR_MAX_ID_LEN];
> > 1553 const struct flash_info *info;
> > 1554
> > 1555 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510804: Integer handling issues
> (CONSTANT_EXPRESSION_RESULT)
> > >>> In "nor->spi->flags |= 256 /* 1 << 8 */", wider "256 /* 1 << 8 */" has high-
> order bits (0x100) that don't affect the narrower left-hand side.
> > 1556 nor->spi->flags |= SPI_XFER_LOWER;
> > 1557
> > 1558 tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> > SPI_NOR_MAX_ID_LEN);
> > 1559 if (tmp < 0) {
> > 1560 dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> > 1561 return ERR_PTR(tmp);
> >
> > ** CID 510803: Code maintainability issues (UNUSED_VALUE)
> > /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510803: Code maintainability issues (UNUSED_VALUE)
> > /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> > 1132 offset = addr;
> > 1133 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 1134 offset /= 2;
> > 1135
> > 1136 if (nor->flags & SNOR_F_HAS_STACKED) {
> > 1137 if (offset >= (mtd->size / 2)) {
> > >>> CID 510803: Code maintainability issues (UNUSED_VALUE)
> > >>> Assigning value from "offset - mtd->size / 2ULL" to "offset" here, but that
> stored value is overwritten before it can be used.
> > 1138 offset = offset - (mtd->size / 2);
> > 1139 nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1140 } else {
> > 1141 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1142 }
> > 1143 }
> >
> > ** CID 510802: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510802: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> > 498 * discard the second byte.
> > 499 */
> > 500 if (spi_nor_protocol_is_dtr(nor->reg_proto))
> > 501 op.data.nbytes = 2;
> > 502
> > 503 if (nor->flags & SNOR_F_HAS_PARALLEL) {
> > >>> CID 510802: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "op.data.nbytes = 2U;".
> > 504 op.data.nbytes = 2;
> > 505 ret = spi_nor_read_write_reg(nor, &op, &val[0]);
> > 506 if (ret < 0) {
> > 507 pr_debug("error %d reading SR\n", (int)ret);
> > 508 return ret;
> > 509 }
> >
> > ** CID 510801: Null pointer dereferences (FORWARD_NULL)
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510801: Null pointer dereferences (FORWARD_NULL)
> > /lib/ecdsa/ecdsa-libcrypto.c: 365 in ecdsa_add_verify_data()
> > 359 struct signer ctx;
> > 360 int ret;
> > 361
> > 362 fdt_key_name = info->keyname ? info->keyname : "default-key";
> > 363 ret = prepare_ctx(&ctx, info);
> > 364 if (ret >= 0) {
> > >>> CID 510801: Null pointer dereferences (FORWARD_NULL)
> > >>> Passing "info" to "do_add", which dereferences null "info->keyname".
> > 365 ret = do_add(&ctx, fdt, fdt_key_name, info);
> > 366 if (ret < 0)
> > 367 ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> > 368 }
> > 369
> > 370 free_ctx(&ctx);
> > 371 return ret;
> >
> > ** CID 510800: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510800: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> > 1614 } else {
> > 1615 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1616 }
> > 1617 }
> > 1618
> > 1619 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510800: (DEADCODE)
> > >>> Execution cannot reach this statement: "offset /= 2LL;".
> > 1620 offset /= 2;
> > 1621
> > 1622 if (nor->addr_width == 3) {
> > 1623 #ifdef CONFIG_SPI_FLASH_BAR
> > 1624 ret = write_bar(nor, offset);
> > 1625 if (ret < 0)
> > /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> > 1584 u32 rem_bank_len = 0;
> > 1585 u8 bank;
> > 1586 bool is_ofst_odd = false;
> > 1587
> > 1588 dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> > 1589
> > >>> CID 510800: (DEADCODE)
> > >>> Execution cannot reach the expression "offset & 1LL" inside this statement:
> "if (nor->flags & SNOR_F_HAS...".
> > 1590 if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) {
> > 1591 /* We can hit this case when we use file system
> > like ubifs */
> > 1592 from--;
> > 1593 len++;
> > 1594 is_ofst_odd = true;
> > 1595 }
> > /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> > 1605 rem_bank_len = (SZ_16M * (bank
> > + 1)) - from;
> > 1606 }
> > 1607 }
> > 1608 offset = from;
> > 1609
> > 1610 if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>> CID 510800: (DEADCODE)
> > >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 1611 if (offset >= (mtd->size / 2)) {
> > 1612 offset = offset - (mtd->size / 2);
> > 1613 nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1614 } else {
> > 1615 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1616 }
> > /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> > 1594 is_ofst_odd = true;
> > 1595 }
> > 1596
> > 1597 while (len) {
> > 1598 if (nor->addr_width == 3) {
> > 1599 if (nor->flags & SNOR_F_HAS_PARALLEL) {
> > >>> CID 510800: (DEADCODE)
> > >>> Execution cannot reach this statement: "bank = (u32)from / 33554432U;".
> > 1600 bank = (u32)from / (SZ_16M << 0x01);
> > 1601 rem_bank_len = ((SZ_16M << 0x01) *
> > 1602 (bank + 1)) - from;
> > 1603 } else {
> > 1604 bank = (u32)from / SZ_16M;
> > 1605 rem_bank_len = (SZ_16M * (bank
> > + 1)) - from;
> >
> > ** CID 510799: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> > /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> > /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510799: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> > 1965 return 0;
> > 1966
> > 1967 /*
> > 1968 * Cannot write to odd offset in parallel mode,
> > 1969 * so write 2 bytes first
> > 1970 */
> > >>> CID 510799: (DEADCODE)
> > >>> Execution cannot reach the expression "to & 1LL" inside this statement: "if
> (nor->flags & SNOR_F_HAS...".
> > 1971 if ((nor->flags & SNOR_F_HAS_PARALLEL) && (to & 1)) {
> > 1972 u8 two[2] = {0xff, buf[0]};
> > 1973 size_t local_retlen;
> > 1974
> > 1975 ret = spi_nor_write(mtd, to & ~1, 2,
> > &local_retlen, two);
> > 1976 if (ret < 0)
> > /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> > 2001 }
> > 2002 offset = (to + i);
> > 2003 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 2004 offset /= 2;
> > 2005
> > 2006 if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>> CID 510799: (DEADCODE)
> > >>> Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 2007 if (offset >= (mtd->size / 2)) {
> > 2008 offset = offset - (mtd->size / 2);
> > 2009 nor->spi->flags |= SPI_XFER_U_PAGE;
> > 2010 } else {
> > 2011 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 2012 }
> > /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> > 1998 u64 aux = addr;
> > 1999
> > 2000 page_offset = do_div(aux, nor->page_size);
> > 2001 }
> > 2002 offset = (to + i);
> > 2003 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510799: (DEADCODE)
> > >>> Execution cannot reach this statement: "offset /= 2U;".
> > 2004 offset /= 2;
> > 2005
> > 2006 if (nor->flags & SNOR_F_HAS_STACKED) {
> > 2007 if (offset >= (mtd->size / 2)) {
> > 2008 offset = offset - (mtd->size / 2);
> > 2009 nor->spi->flags |= SPI_XFER_U_PAGE;
> >
> > ** CID 510798: Resource leaks (RESOURCE_LEAK)
> > /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510798: Resource leaks (RESOURCE_LEAK)
> > /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> > 214 }
> > 215
> > 216 ret = hash_calculate(s->hash_algo, ®ion, 1, s->digest);
> > 217 if (!ret)
> > 218 *sig = s;
> > 219
> > >>> CID 510798: Resource leaks (RESOURCE_LEAK)
> > >>> Variable "s" going out of scope leaks the storage it points to.
> > 220 return ret;
> > 221
> > 222 error_sig:
> > 223 public_key_signature_free(s);
> > 224 return ret;
> > 225 }
> >
> > ** CID 510797: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> > /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510797: (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> > 4622 /* Send all the required SPI flash commands to
> > initialize device */
> > 4623 ret = spi_nor_init(nor);
> > 4624 if (ret)
> > 4625 return ret;
> > 4626
> > 4627 if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>> CID 510797: (DEADCODE)
> > >>> Execution cannot reach this statement: "nor->spi->flags |= 0x10UL;".
> > 4628 nor->spi->flags |= SPI_XFER_U_PAGE;
> > 4629 ret = spi_nor_init(nor);
> > 4630 if (ret)
> > 4631 return ret;
> > 4632 nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 4633 }
> > /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> > 4592 nor->addr_width = info->addr_width;
> > 4593 } else {
> > 4594 nor->addr_width = 3;
> > 4595 }
> > 4596
> > 4597 if (nor->flags & (SNOR_F_HAS_PARALLEL |
> SNOR_F_HAS_STACKED))
> > >>> CID 510797: (DEADCODE)
> > >>> Execution cannot reach this statement: "shift = true;".
> > 4598 shift = 1;
> > 4599 if (nor->addr_width == 3 && (mtd->size >> shift) > SZ_16M) {
> > 4600 #ifndef CONFIG_SPI_FLASH_BAR
> > 4601 /* enable 4-byte addressing if the device
> > exceeds 16MiB */
> > 4602 nor->addr_width = 4;
> > 4603 if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> >
> > ** CID 510796: Insecure data handling (TAINTED_SCALAR)
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510796: Insecure data handling (TAINTED_SCALAR)
> > /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> > 1310 }
> > 1311
> > 1312 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> > ctx->len - 1, f_rng, p_rng));
> > 1313
> > 1314 /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> > inv_mod. */
> > 1315 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> > - 1, f_rng, p_rng));
> > >>> CID 510796: Insecure data handling (TAINTED_SCALAR)
> > >>> Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi", which
> uses it as an offset.
> > 1316 MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf,
> &R));
> > 1317 MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> > &ctx->Vi, &ctx->N));
> > 1318
> > 1319 /* At this point, Vi is invertible mod N if and only
> > if both Vf and R
> > 1320 * are invertible mod N. If one of them isn't, we
> > don't need to know
> > 1321 * which one, we just loop and choose new values for
> > both of them.
> >
> > ** CID 510795: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510795: Control flow issues (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> > 4265
> > 4266 static int spi_nor_init(struct spi_nor *nor)
> > 4267 {
> > 4268 int err;
> > 4269
> > 4270 if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>> CID 510795: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "nor->spi->flags |= 3UL;".
> > 4271 nor->spi->flags |= SPI_NOR_ENABLE_MULTI_CS;
> > 4272
> > 4273 err = spi_nor_octal_dtr_enable(nor);
> > 4274 if (err) {
> > 4275 dev_dbg(nor->dev, "Octal DTR mode not supported\n");
> > 4276 return err;
> >
> > ** CID 510794: Control flow issues (NO_EFFECT)
> > /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510794: Control flow issues (NO_EFFECT)
> > /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> > 72 do {
> > 73 name_str = kzalloc(len, GFP_KERNEL);
> > 74 if (!name_str)
> > 75 return NULL;
> > 76
> > 77 wb = mbedtls_x509_dn_gets(name_str, len, name);
> > >>> CID 510794: Control flow issues (NO_EFFECT)
> > >>> This less-than-zero comparison of an unsigned value is never true. "wb <
> 0UL".
> > 78 if (wb < 0) {
> > 79 pr_err("Get DN string failed, ret:-0x%04x\n",
> > 80 (unsigned int)-wb);
> > 81 kfree(name_str);
> > 82 len = len * 2; /* Try with a bigger buffer */
> > 83 }
> >
> > ----- End forwarded message -----
> >
> > --
> > Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2024-10-16 3:47 Fwd: " Tom Rini
2024-10-16 6:12 ` Ilias Apalodimas
@ 2024-10-16 15:23 ` Raymond Mao
1 sibling, 0 replies; 19+ messages in thread
From: Raymond Mao @ 2024-10-16 15:23 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
Venkatesh Yadav Abbarapu, Pratyush Yadav, Ashok Reddy Soma,
Joakim Tjernlund, Ilias Apalodimas
Hi Tom,
On Tue, 15 Oct 2024 at 23:47, Tom Rini <trini@konsulko.com> wrote:
> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 20 of 22 defect(s)
>
> [snip]
>
> *** CID 510809: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> 379 signed_info);
> 380 if (ret)
> 381 goto out_err_sinfo;
> 382
> 383 no_authattrs:
> 384 *sinfo = signed_info;
> >>> CID 510809: Resource leaks (RESOURCE_LEAK)
> >>> Variable "mctx" going out of scope leaks the storage it points to.
> 385 return 0;
> 386
> 387 out_err_sinfo:
> 388 pkcs7_free_sinfo_mbedtls_ctx(mctx);
> 389 out_no_mctx:
> 390 public_key_signature_free(s);
>
> I will post a patch to fix this defect .
[snip]
> *** CID 510807: Control flow issues (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in
> x509_inet_pton_ipv6()
> 2744 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> 2745 nonzero_groups++;
> 2746 if (*p == '\0') {
> 2747 break;
> 2748 } else if (*p == '.') {
> 2749 /* Don't accept IPv4 too early or late */
> >>> CID 510807: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "zero_group_start == -1"
> inside this statement: "if ((nonzero_groups == 0 &&...".
> 2750 if ((nonzero_groups == 0 && zero_group_start ==
> -1) ||
> 2751 nonzero_groups >= 7) {
> 2752 break;
> 2753 }
> 2754
> 2755 /* Walk back to prior ':', then parse as
> IPv4-mapped */
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]
*** CID 510806: Control flow issues (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> 203 return -EINVAL;
> 204 }
> 205
> 206 p += seq_len;
> 207 }
> 208
> >>> CID 510806: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "ret != -96" inside this
> statement: "if (ret && ret != -96)
> re...".
> 209 if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> 210 return ret;
> 211
> 212 msg->have_authattrs = true;
> 213
> 214 /*
>
> I will post a patch to fix this defect .
[snip]
> *** CID 510798: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> 214 }
> 215
> 216 ret = hash_calculate(s->hash_algo, ®ion, 1, s->digest);
> 217 if (!ret)
> 218 *sig = s;
> 219
> >>> CID 510798: Resource leaks (RESOURCE_LEAK)
> >>> Variable "s" going out of scope leaks the storage it points to.
> 220 return ret;
> 221
> 222 error_sig:
> 223 public_key_signature_free(s);
> 224 return ret;
> 225 }
>
> This is false-positive, we need to keep the memory pointed by 's' as the
signature
returned to the caller.
[snip]
*** CID 510796: Insecure data handling (TAINTED_SCALAR)
> /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> 1310 }
> 1311
> 1312 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> ctx->len - 1, f_rng, p_rng));
> 1313
> 1314 /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> inv_mod. */
> 1315 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> - 1, f_rng, p_rng));
> >>> CID 510796: Insecure data handling (TAINTED_SCALAR)
> >>> Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi",
> which uses it as an offset.
> 1316 MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf,
> &R));
> 1317 MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> &ctx->Vi, &ctx->N));
> 1318
> 1319 /* At this point, Vi is invertible mod N if and only
> if both Vf and R
> 1320 * are invertible mod N. If one of them isn't, we
> don't need to know
> 1321 * which one, we just loop and choose new values for
> both of them.
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]
*** CID 510794: Control flow issues (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> 72 do {
> 73 name_str = kzalloc(len, GFP_KERNEL);
> 74 if (!name_str)
> 75 return NULL;
> 76
> 77 wb = mbedtls_x509_dn_gets(name_str, len, name);
> >>> CID 510794: Control flow issues (NO_EFFECT)
> >>> This less-than-zero comparison of an unsigned value is never true.
> "wb < 0UL".
> 78 if (wb < 0) {
> 79 pr_err("Get DN string failed, ret:-0x%04x\n",
> 80 (unsigned int)-wb);
> 81 kfree(name_str);
> 82 len = len * 2; /* Try with a bigger buffer */
> 83 }
>
> I will post a patch to fix this defect.
Thanks a lot for the report.
Regards,
Raymond
^ permalink raw reply [flat|nested] 19+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-12-31 13:55 Tom Rini
2025-01-01 10:50 ` Abbarapu, Venkatesh
0 siblings, 1 reply; 19+ messages in thread
From: Tom Rini @ 2024-12-31 13:55 UTC (permalink / raw)
To: u-boot, Venkatesh Yadav Abbarapu
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
Hey all, here's the latest report.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Dec 30, 2024, 10:44 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To: <tom.rini@gmail.com>
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot
found with Coverity Scan.
2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
1 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 2 of 2 defect(s)
** CID 528528: Control flow issues (DEADCODE)
/drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
________________________________________________________________________________________________________
*** CID 528528: Control flow issues (DEADCODE)
/drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
1638 read_len = len;
1639 else
1640 read_len = rem_bank_len;
1641 #endif
1642
1643 if (read_len == 0)
>>> CID 528528: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return -5;".
1644 return -EIO;
1645
1646 ret = nor->read(nor, offset, read_len, buf);
1647 if (ret == 0) {
1648 /* We shouldn't see 0-length reads */
1649 ret = -EIO;
** CID 528527: Code maintainability issues (UNUSED_VALUE)
/drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
________________________________________________________________________________________________________
*** CID 528527: Code maintainability issues (UNUSED_VALUE)
/drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
1607 }
1608 rem_bank_len = SZ_16M * (bank + 1);
1609 if
(CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
1610 if (nor->flags &
SNOR_F_HAS_PARALLEL)
1611 rem_bank_len *= 2;
1612 }
>>> CID 528527: Code maintainability issues (UNUSED_VALUE)
>>> Assigning value from "rem_bank_len - from" to "rem_bank_len" here,
but that stored value is overwritten before it can be used.
1613 rem_bank_len -= from;
1614 }
1615
1616 if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
1617 if (nor->flags & SNOR_F_HAS_STACKED) {
1618 stack_shift = 1;
----- End forwarded message -----
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: New Defects reported by Coverity Scan for Das U-Boot
2024-12-31 13:55 Fwd: New Defects reported by Coverity Scan for Das U-Boot Tom Rini
@ 2025-01-01 10:50 ` Abbarapu, Venkatesh
2025-01-02 16:59 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Abbarapu, Venkatesh @ 2025-01-01 10:50 UTC (permalink / raw)
To: Tom Rini, u-boot@lists.denx.de
Hi Tom,
These defects won't appear when SPI_STACKED_PARALLEL config is enabled. We can mark these as false positives.
Thanks
Venkatesh
> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: Tuesday, December 31, 2024 7:25 PM
> To: u-boot@lists.denx.de; Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>
> Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot
>
> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Dec 30, 2024, 10:44 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot found with
> Coverity Scan.
>
> 2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 1 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 2 of 2 defect(s)
>
>
> ** CID 528528: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
>
>
> ___________________________________________________________________
> _____________________________________
> *** CID 528528: Control flow issues (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
> 1638 read_len = len;
> 1639 else
> 1640 read_len = rem_bank_len;
> 1641 #endif
> 1642
> 1643 if (read_len == 0)
> >>> CID 528528: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "return -5;".
> 1644 return -EIO;
> 1645
> 1646 ret = nor->read(nor, offset, read_len, buf);
> 1647 if (ret == 0) {
> 1648 /* We shouldn't see 0-length reads */
> 1649 ret = -EIO;
>
> ** CID 528527: Code maintainability issues (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
>
>
> ___________________________________________________________________
> _____________________________________
> *** CID 528527: Code maintainability issues (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
> 1607 }
> 1608 rem_bank_len = SZ_16M * (bank + 1);
> 1609 if
> (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> 1610 if (nor->flags &
> SNOR_F_HAS_PARALLEL)
> 1611 rem_bank_len *= 2;
> 1612 }
> >>> CID 528527: Code maintainability issues (UNUSED_VALUE)
> >>> Assigning value from "rem_bank_len - from" to "rem_bank_len"
> >>> here,
> but that stored value is overwritten before it can be used.
> 1613 rem_bank_len -= from;
> 1614 }
> 1615
> 1616 if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> 1617 if (nor->flags & SNOR_F_HAS_STACKED) {
> 1618 stack_shift = 1;
>
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-01-01 10:50 ` Abbarapu, Venkatesh
@ 2025-01-02 16:59 ` Tom Rini
0 siblings, 0 replies; 19+ messages in thread
From: Tom Rini @ 2025-01-02 16:59 UTC (permalink / raw)
To: Abbarapu, Venkatesh; +Cc: u-boot@lists.denx.de
[-- Attachment #1: Type: text/plain, Size: 223 bytes --]
On Wed, Jan 01, 2025 at 10:50:02AM +0000, Abbarapu, Venkatesh wrote:
> Hi Tom,
> These defects won't appear when SPI_STACKED_PARALLEL config is enabled. We can mark these as false positives.
OK, thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-02-10 22:26 Fwd: " Tom Rini
@ 2025-02-11 22:24 ` Raymond Mao
2025-02-11 22:30 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Raymond Mao @ 2025-02-11 22:24 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, Heiko Schocher, Ilias Apalodimas
Hi Tom,
On Mon, 10 Feb 2025 at 17:26, Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Feb 10, 2025 at 4:12 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot
> found with Coverity Scan.
>
> 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 3 of 3 defect(s)
>
>
> ** CID 541281: Insecure data handling (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
>
>
> ________________________________________________________________________________________________________
> *** CID 541281: Insecure data handling (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> 71 *mask = 0;
> 72
> 73 rc = tpm2_get_pcr_info(dev, &pcrs);
> 74 if (rc)
> 75 return rc;
> 76
> >>> CID 541281: Insecure data handling (TAINTED_SCALAR)
> >>> Using tainted variable "pcrs.count" as a loop boundary.
We don't need to check the pcrs.count here, since tpm2_get_pcr_info()
will post an error if pcrs.count is not in a value from 1 to 4.
This is hardcoded in tpm2_get_pcr_info(), please see below:
```
/*
* We only support 4 algorithms for now so check against that
* instead of TPM2_NUM_PCR_BANKS
*/
if (pcrs->count > 4 || pcrs->count < 1) {
printf("%s: too many pcrs: %u\n", __func__, pcrs->count);
return -EMSGSIZE;
}
```
> 77 for (i = 0; i < pcrs.count; i++) {
> 78 struct tpms_pcr_selection *sel = &pcrs.selection[i];
> 79 size_t j;
> 80 u32 hash_mask = 0;
> 81
> 82 for (j = 0; j < ARRAY_SIZE(hash_algo_list); j++) {
>
> ** CID 541280: Insecure data handling (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
>
>
> ________________________________________________________________________________________________________
> *** CID 541280: Insecure data handling (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> 301 * first call
> 302 */
> 303 ret = tpm2_get_pcr_info(dev, &pcr);
> 304 if (ret)
> 305 return ret;
> 306
> >>> CID 541280: Insecure data handling (TAINTED_SCALAR)
> >>> Using tainted variable "pcr.count" as a loop boundary.
Ditto.
Regards,
Raymond
> 307 for (i = 0; i < pcr.count; i++) {
> 308 struct tpms_pcr_selection *sel =
> &pcr.selection[i];
> 309 const char *name;
> 310
> 311 if (!tpm2_is_active_bank(sel))
> 312 continue;
>
> ** CID 541279: (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
>
>
> ________________________________________________________________________________________________________
> *** CID 541279: (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> 278 if (!ret) {
> 279 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280 "%s:%s-%d",
> 281 cp ? "" : led_colors[color],
> 282 func ? func : "", enumerator);
> 283 } else {
> >>> CID 541279: (TAINTED_SCALAR)
> >>> Using tainted variable "color" as an index into an array
> "led_colors".
> 284 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 285 "%s:%s",
> 286 cp ? "" : led_colors[color],
> 287 func ? func : "");
> 288 }
> 289 uc_plat->label = uc_plat->name;
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
> 273 /* Now try to detect function label name */
> 274 func = dev_read_string(dev, "function");
> 275 cp = dev_read_u32(dev, "color", &color);
> 276 if (cp == 0 || func) {
> 277 ret = dev_read_u32(dev, "function-enumerator",
> &enumerator);
> 278 if (!ret) {
> >>> CID 541279: (TAINTED_SCALAR)
> >>> Using tainted variable "color" as an index into an array
> "led_colors".
> 279 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280 "%s:%s-%d",
> 281 cp ? "" : led_colors[color],
> 282 func ? func : "", enumerator);
> 283 } else {
> 284 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
>
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-02-11 22:24 ` Raymond Mao
@ 2025-02-11 22:30 ` Tom Rini
0 siblings, 0 replies; 19+ messages in thread
From: Tom Rini @ 2025-02-11 22:30 UTC (permalink / raw)
To: Raymond Mao; +Cc: u-boot, Heiko Schocher, Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]
On Tue, Feb 11, 2025 at 05:24:02PM -0500, Raymond Mao wrote:
> Hi Tom,
>
> On Mon, 10 Feb 2025 at 17:26, Tom Rini <trini@konsulko.com> wrote:
> >
> > Here's the latest report.
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Feb 10, 2025 at 4:12 PM
> > Subject: New Defects reported by Coverity Scan for Das U-Boot
> > To: <tom.rini@gmail.com>
> >
> >
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to Das U-Boot
> > found with Coverity Scan.
> >
> > 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 3 of 3 defect(s)
> >
> >
> > ** CID 541281: Insecure data handling (TAINTED_SCALAR)
> > /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 541281: Insecure data handling (TAINTED_SCALAR)
> > /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> > 71 *mask = 0;
> > 72
> > 73 rc = tpm2_get_pcr_info(dev, &pcrs);
> > 74 if (rc)
> > 75 return rc;
> > 76
> > >>> CID 541281: Insecure data handling (TAINTED_SCALAR)
> > >>> Using tainted variable "pcrs.count" as a loop boundary.
>
> We don't need to check the pcrs.count here, since tpm2_get_pcr_info()
> will post an error if pcrs.count is not in a value from 1 to 4.
> This is hardcoded in tpm2_get_pcr_info(), please see below:
> ```
> /*
> * We only support 4 algorithms for now so check against that
> * instead of TPM2_NUM_PCR_BANKS
> */
> if (pcrs->count > 4 || pcrs->count < 1) {
> printf("%s: too many pcrs: %u\n", __func__, pcrs->count);
> return -EMSGSIZE;
> }
> ```
>
> > 77 for (i = 0; i < pcrs.count; i++) {
> > 78 struct tpms_pcr_selection *sel = &pcrs.selection[i];
> > 79 size_t j;
> > 80 u32 hash_mask = 0;
> > 81
> > 82 for (j = 0; j < ARRAY_SIZE(hash_algo_list); j++) {
> >
> > ** CID 541280: Insecure data handling (TAINTED_SCALAR)
> > /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 541280: Insecure data handling (TAINTED_SCALAR)
> > /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> > 301 * first call
> > 302 */
> > 303 ret = tpm2_get_pcr_info(dev, &pcr);
> > 304 if (ret)
> > 305 return ret;
> > 306
> > >>> CID 541280: Insecure data handling (TAINTED_SCALAR)
> > >>> Using tainted variable "pcr.count" as a loop boundary.
>
> Ditto.
OK, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-07-08 14:10 Fwd: " Tom Rini
@ 2025-07-09 9:13 ` Sughosh Ganu
0 siblings, 0 replies; 19+ messages in thread
From: Sughosh Ganu @ 2025-07-09 9:13 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Simon Glass, Heinrich Schuchardt, Ilias Apalodimas,
Marek Vasut, Ying-Chun Liu (PaulLiu), Aristo Chen,
Rasmus Villemoes, Sean Edmond, Miquel Raynal
On Tue, 8 Jul 2025 at 19:40, Tom Rini <trini@konsulko.com> wrote:
>
> Hey all,
>
> Good news, Coverity Scan resumed putting information in the email
> report. Bad news, 20 new issues now that next has been merged.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Jul 7, 2025 at 5:39 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot*
> found with Coverity Scan.
>
> - *New Defects Found:* 20
> - 6 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 20 of 20 defect(s)
>
> Defect Details
>
[...]
> _____________________________________________________________________________________________
> *** CID 569481: Control flow issues (MISSING_BREAK)
> /lib/lmb.c: 763 in lmb_alloc_mem()
> 757 return 0;
> 758
> 759 if (!addr)
> 760 return -EINVAL;
> 761
> 762 switch (type) {
> >>> CID 569481: Control flow issues (MISSING_BREAK)
> >>> The case for value "LMB_MEM_ALLOC_ANY" is not terminated by a "break" statement.
The missing break is on purpose, so this is not an issue. Nonetheless,
Heinrich has sent a patch [1] to put a fallthrough statement here.
Thanks.
-sughosh
[1] - https://patchwork.ozlabs.org/project/uboot/patch/20250708121251.83980-1-heinrich.schuchardt@canonical.com/
> 763 case LMB_MEM_ALLOC_ANY:
> 764 *addr = LMB_ALLOC_ANYWHERE;
> 765 case LMB_MEM_ALLOC_MAX:
> 766 ret = _lmb_alloc_base(size, align, addr, flags);
> 767 break;
> 768 case LMB_MEM_ALLOC_ADDR:
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: New Defects reported by Coverity Scan for Das U-Boot
2025-08-06 18:35 Fwd: " Tom Rini
@ 2025-08-07 1:50 ` Maniyam, Dinesh
0 siblings, 0 replies; 19+ messages in thread
From: Maniyam, Dinesh @ 2025-08-07 1:50 UTC (permalink / raw)
To: Tom Rini, u-boot@lists.denx.de, Heiko Schocher
Hi Tom
> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: Thursday, 7 August 2025 2:36 am
> To: u-boot@lists.denx.de; Heiko Schocher <hs@denx.de>; Maniyam, Dinesh
> <dinesh.maniyam@altera.com>
> Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot
>
> Here's the latest report. Lets get these new issues addressed ASAP please,
> thanks.
I will work on resolving the issues.
Thanks!
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Wed, Aug 6, 2025 at 12:23 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot* found
> with Coverity Scan.
>
> - *New Defects Found:* 8
> - 4 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 8 of 8 defect(s)
>
> Defect Details
>
> ** CID 583812: Integer handling issues (BAD_SHIFT)
> /drivers/i3c/master/dw-i3c-master.c: 1001 in dw_i3c_probe()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 583812: Integer handling issues (BAD_SHIFT)
> /drivers/i3c/master/dw-i3c-master.c: 1001 in dw_i3c_probe()
> 995 ret = readl(master->regs + DATA_BUFFER_STATUS_LEVEL);
> 996 master->caps.datafifodepth = DATA_BUFFER_STATUS_LEVEL_TX(ret);
> 997
> 998 ret = readl(master->regs + DEVICE_ADDR_TABLE_POINTER);
> 999 master->datstartaddr = ret;
> 1000 master->maxdevs = ret >> 16;
> >>> CID 583812: Integer handling issues (BAD_SHIFT)
> >>> In expression "0xffffffffffffffffUL >> 63 - (master->maxdevs - 1)", right
> shifting by more than 63 bits has undefined behavior. The shift amount, "63 -
> (master->maxdevs - 1)", is 64.
> 1001 master->free_pos = GENMASK(master->maxdevs - 1, 0);
> 1002
> 1003 ret = i3c_master_register(&master->base, dev,
> 1004 &dw_mipi_i3c_ops, false);
> 1005 if (ret)
> 1006 goto err_assert_rst;
>
> ** CID 583811: (RESOURCE_LEAK)
> /drivers/i3c/master.c: 1610 in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1586 in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1591 in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1598 in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1603 in of_i3c_master_add_i3c_boardinfo()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 583811: (RESOURCE_LEAK)
> /drivers/i3c/master.c: 1610 in of_i3c_master_add_i3c_boardinfo()
> 1604 }
> 1605
> 1606 boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> 1607
> 1608 if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> 1609 I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> >>> CID 583811: (RESOURCE_LEAK)
> >>> Variable "boardinfo" going out of scope leaks the storage it points to.
> 1610 return -EINVAL;
> 1611
> 1612 boardinfo->init_dyn_addr = init_dyn_addr;
> 1613 boardinfo->of_node = node;
> 1614 list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
> 1615
> /drivers/i3c/master.c: 1586 in of_i3c_master_add_i3c_boardinfo()
> 1580 boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
> 1581 if (!boardinfo)
> 1582 return -ENOMEM;
> 1583
> 1584 if (reg[0]) {
> 1585 if (reg[0] > I3C_MAX_ADDR)
> >>> CID 583811: (RESOURCE_LEAK)
> >>> Variable "boardinfo" going out of scope leaks the storage it points to.
> 1586 return -EINVAL;
> 1587
> 1588 addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1589 reg[0]);
> 1590 if (addrstatus != I3C_ADDR_SLOT_FREE)
> 1591 return -EINVAL;
> /drivers/i3c/master.c: 1591 in of_i3c_master_add_i3c_boardinfo()
> 1585 if (reg[0] > I3C_MAX_ADDR)
> 1586 return -EINVAL;
> 1587
> 1588 addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1589 reg[0]);
> 1590 if (addrstatus != I3C_ADDR_SLOT_FREE)
> >>> CID 583811: (RESOURCE_LEAK)
> >>> Variable "boardinfo" going out of scope leaks the storage it points to.
> 1591 return -EINVAL;
> 1592 }
> 1593
> 1594 boardinfo->static_addr = reg[0];
> 1595
> 1596 if (!dev_read_u32(dev, "assigned-address", &init_dyn_addr)) {
> /drivers/i3c/master.c: 1598 in of_i3c_master_add_i3c_boardinfo()
> 1592 }
> 1593
> 1594 boardinfo->static_addr = reg[0];
> 1595
> 1596 if (!dev_read_u32(dev, "assigned-address", &init_dyn_addr)) {
> 1597 if (init_dyn_addr > I3C_MAX_ADDR)
> >>> CID 583811: (RESOURCE_LEAK)
> >>> Variable "boardinfo" going out of scope leaks the storage it points to.
> 1598 return -EINVAL;
> 1599
> 1600 addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1601 init_dyn_addr);
> 1602 if (addrstatus != I3C_ADDR_SLOT_FREE)
> 1603 return -EINVAL;
> /drivers/i3c/master.c: 1603 in of_i3c_master_add_i3c_boardinfo()
> 1597 if (init_dyn_addr > I3C_MAX_ADDR)
> 1598 return -EINVAL;
> 1599
> 1600 addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1601 init_dyn_addr);
> 1602 if (addrstatus != I3C_ADDR_SLOT_FREE)
> >>> CID 583811: (RESOURCE_LEAK)
> >>> Variable "boardinfo" going out of scope leaks the storage it points to.
> 1603 return -EINVAL;
> 1604 }
> 1605
> 1606 boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> 1607
> 1608 if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
>
> ** CID 298388: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 579 in dw_i3c_ccc_get()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 298388: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 579 in dw_i3c_ccc_get()
> 573 return -ENOMEM;
> 574
> 575 cmd = xfer->cmds;
> 576 cmd->rx_buf = ccc->dests[0].payload.data;
> 577 cmd->rx_len = ccc->dests[0].payload.len;
> 578
> >>> CID 298388: Integer handling issues (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "ccc->dests[0].payload.len" with type
> "u16" (16 bits, unsigned) is promoted in "ccc->dests[0].payload.len << 16" to type
> "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits,
> unsigned). If "ccc->dests[0].payload.len << 16" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 579 cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(ccc-
> >dests[0].payload.len) |
> 580 COMMAND_PORT_TRANSFER_ARG;
> 581
> 582 cmd->cmd_lo = COMMAND_PORT_READ_TRANSFER |
> 583 COMMAND_PORT_CP |
> 584 COMMAND_PORT_DEV_INDEX(pos) |
>
> ** CID 298037: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 375 in dw_i3c_clk_cfg()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 298037: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 375 in dw_i3c_clk_cfg()
> 369 scl_timing = SCL_EXT_LCNT_1(lcnt);
> 370 lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR2_SCL_RATE) - hcnt;
> 371 scl_timing |= SCL_EXT_LCNT_2(lcnt);
> 372 lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR3_SCL_RATE) - hcnt;
> 373 scl_timing |= SCL_EXT_LCNT_3(lcnt);
> 374 lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR4_SCL_RATE) - hcnt;
> >>> CID 298037: Integer handling issues (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "lcnt" with type "u8" (8 bits, unsigned) is
> promoted in "lcnt << 24" to type "int" (32 bits, signed), then sign-extended to type
> "unsigned long" (64 bits, unsigned). If "lcnt << 24" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 375 scl_timing |= SCL_EXT_LCNT_4(lcnt);
> 376 writel(scl_timing, master->regs + SCL_EXT_LCNT_TIMING);
> 377
> 378 return 0;
> 379 }
> 380
>
> ** CID 296053: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 535 in dw_i3c_ccc_set()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 296053: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 535 in dw_i3c_ccc_set()
> 529 return -ENOMEM;
> 530
> 531 cmd = xfer->cmds;
> 532 cmd->tx_buf = ccc->dests[0].payload.data;
> 533 cmd->tx_len = ccc->dests[0].payload.len;
> 534
> >>> CID 296053: Integer handling issues (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "ccc->dests[0].payload.len" with type
> "u16" (16 bits, unsigned) is promoted in "ccc->dests[0].payload.len << 16" to type
> "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits,
> unsigned). If "ccc->dests[0].payload.len << 16" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 535 cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(ccc-
> >dests[0].payload.len) |
> 536 COMMAND_PORT_TRANSFER_ARG;
> 537
> 538 cmd->cmd_lo = COMMAND_PORT_CP |
> 539 COMMAND_PORT_DEV_INDEX(pos) |
> 540 COMMAND_PORT_CMD(ccc->id) |
>
> ** CID 295976: (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 395 in dw_i2c_clk_cfg()
> /drivers/i3c/master/dw-i3c-master.c: 401 in dw_i2c_clk_cfg()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 295976: (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 395 in dw_i2c_clk_cfg()
> 389 return -EINVAL;
> 390
> 391 core_period = DIV_ROUND_UP(1000000000, core_rate);
> 392
> 393 lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FMP_TLOW_MIN_NS,
> core_period);
> 394 hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_PLUS_SCL_RATE) -
> lcnt;
> >>> CID 295976: (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "hcnt" with type "u16" (16 bits,
> unsigned) is promoted in "hcnt << 16" to type "int" (32 bits, signed), then sign-
> extended to type "unsigned long" (64 bits, unsigned). If "hcnt << 16" is greater
> than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 395 scl_timing = SCL_I2C_FMP_TIMING_HCNT(hcnt) |
> 396 SCL_I2C_FMP_TIMING_LCNT(lcnt);
> 397 writel(scl_timing, master->regs + SCL_I2C_FMP_TIMING);
> 398
> 399 lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FM_TLOW_MIN_NS, core_period);
> 400 hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_SCL_RATE) - lcnt;
> /drivers/i3c/master/dw-i3c-master.c: 401 in dw_i2c_clk_cfg()
> 395 scl_timing = SCL_I2C_FMP_TIMING_HCNT(hcnt) |
> 396 SCL_I2C_FMP_TIMING_LCNT(lcnt);
> 397 writel(scl_timing, master->regs + SCL_I2C_FMP_TIMING);
> 398
> 399 lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FM_TLOW_MIN_NS, core_period);
> 400 hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_SCL_RATE) - lcnt;
> >>> CID 295976: (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "hcnt" with type "u16" (16 bits,
> unsigned) is promoted in "hcnt << 16" to type "int" (32 bits, signed), then sign-
> extended to type "unsigned long" (64 bits, unsigned). If "hcnt << 16" is greater
> than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 401 scl_timing = SCL_I2C_FM_TIMING_HCNT(hcnt) |
> 402 SCL_I2C_FM_TIMING_LCNT(lcnt);
> 403 writel(scl_timing, master->regs + SCL_I2C_FM_TIMING);
> 404
> 405 writel(BUS_I3C_MST_FREE(lcnt), master->regs + BUS_FREE_TIMING);
> 406 writel(readl(master->regs + DEVICE_CTRL) |
> DEV_CTRL_I2C_SLAVE_PRESENT,
>
> ** CID 294913: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 724 in dw_i3c_master_priv_xfers()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 294913: Integer handling issues (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 724 in
> dw_i3c_master_priv_xfers()
> 718 if (!xfer)
> 719 return -ENOMEM;
> 720
> 721 for (i = 0; i < i3c_nxfers; i++) {
> 722 struct dw_i3c_cmd *cmd = &xfer->cmds[i];
> 723
> >>> CID 294913: Integer handling issues (SIGN_EXTENSION)
> >>> Suspicious implicit sign extension: "i3c_xfers[i].len" with type "u16" (16
> bits, unsigned) is promoted in "i3c_xfers[i].len << 16" to type "int" (32 bits,
> signed), then sign-extended to type "unsigned long" (64 bits, unsigned). If
> "i3c_xfers[i].len << 16" is greater than 0x7FFFFFFF, the upper bits of the result will
> all be 1.
> 724 cmd->cmd_hi =
> COMMAND_PORT_ARG_DATA_LEN(i3c_xfers[i].len) |
> 725 COMMAND_PORT_TRANSFER_ARG;
> 726
> 727 if (i3c_xfers[i].rnw) {
> 728 cmd->rx_buf = i3c_xfers[i].data.in;
> 729 cmd->rx_len = i3c_xfers[i].len;
>
> ** CID 294627: Integer handling issues (BAD_SHIFT)
> /drivers/i3c/master.c: 181 in i3c_bus_get_addr_slot_status()
>
>
> _________________________________________________________________
> ____________________________
> *** CID 294627: Integer handling issues (BAD_SHIFT)
> /drivers/i3c/master.c: 181 in i3c_bus_get_addr_slot_status()
> 175 int status, bitpos = addr * 2;
> 176
> 177 if (addr > I2C_MAX_ADDR)
> 178 return I3C_ADDR_SLOT_RSVD;
> 179
> 180 status = bus->addrslots[bitpos / BITS_PER_LONG];
> >>> CID 294627: Integer handling issues (BAD_SHIFT)
> >>> In expression "status >>= bitpos % 64", right shifting by more than 31 bits
> has undefined behavior. The shift amount, "bitpos % 64", is as much as 63.
> 181 status >>= bitpos % BITS_PER_LONG;
> 182
> 183 return status & I3C_ADDR_SLOT_STATUS_MASK;
> 184 }
> 185
> 186 static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr,
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-11-10 18:55 Fwd: " Tom Rini
@ 2025-11-12 8:53 ` Kory Maincent
0 siblings, 0 replies; 19+ messages in thread
From: Kory Maincent @ 2025-11-12 8:53 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
On Mon, 10 Nov 2025 12:55:07 -0600
Tom Rini <trini@konsulko.com> wrote:
> Here's the latest report. Just 2 new issues, both from the extensions
> series. Can we please address these shortly? Thanks!
Ok I will take a look at these errors!
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Nov 10, 2025 at 12:44 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot*
> found with Coverity Scan.
>
> - *New Defects Found:* 2
> - 1 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 2 of 2 defect(s)
>
> Defect Details
>
> ** CID 638558: Memory - illegal accesses (UNINIT)
> /boot/pxe_utils.c: 485 in label_boot_extension()
>
>
> _____________________________________________________________________________________________
> *** CID 638558: Memory - illegal accesses (UNINIT)
> /boot/pxe_utils.c: 485 in label_boot_extension()
> 479 return;
> 480
> 481 snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir,
> 482 slash);
> 483 } else {
> 484 dir_len = 2;
> >>> CID 638558: Memory - illegal accesses (UNINIT)
> >>> Using uninitialized value "overlay_dir" when calling "snprintf".
> >>> [Note: The source code implementation of the function has been overridden
> >>> by a builtin model.]
> 485 snprintf(overlay_dir, dir_len, "/");
> 486 }
> 487
> 488 alist_for_each(extension, extension_list) {
> 489 char *overlay_file;
> 490 ulong size;
>
> ** CID 638557: Null pointer dereferences (NULL_RETURNS)
>
>
> _____________________________________________________________________________________________
> *** CID 638557: Null pointer dereferences (NULL_RETURNS)
> /cmd/extension_board.c: 102 in do_extension_list()
> 96 {
> 97 struct alist *extension_list;
> 98 struct extension *extension;
> 99 int i = 0;
> 100
> 101 extension_list = extension_get_list();
> >>> CID 638557: Null pointer dereferences (NULL_RETURNS)
> >>> Dereferencing a pointer that might be "NULL" "extension_list" when
> >>> calling "alist_get_ptr".
> 102 if (!alist_get_ptr(extension_list, 0)) {
> 103 printf("No extension registered - Please run
> \"extension scan\"\n"); 104 return CMD_RET_SUCCESS;
> 105 }
> 106
> 107 alist_for_each(extension, extension_list) {
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-12-08 19:38 Fwd: " Tom Rini
@ 2025-12-09 11:06 ` Adriana Nicolae
2025-12-09 14:24 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Adriana Nicolae @ 2025-12-09 11:06 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hello,
A possible fix for fdtdec.c tests would be to validate the fdt size
before using it.
All 3 tests in this file are using the same approach, if the previous tests were
acceptable the newly added one has some identical parts.
If there is a way to test and fix all these 3 errors, I've put some
changes which
might be enough for Coverity to assume that the size of the fdt is safe. Let me
know how I can test it or should I send an email to post it as a
separate change?
diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c
index ea5a494612c..a3c90d38115 100644
--- a/test/dm/fdtdec.c
+++ b/test/dm/fdtdec.c
@@ -14,14 +14,21 @@
DECLARE_GLOBAL_DATA_PTR;
+#define FDTDEC_MAX_SIZE (2 * 1024 * 1024)
+
static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts)
{
struct fdt_memory resv;
void *blob;
const fdt32_t *prop;
- int blob_sz, len, offset;
+ int blob_sz, len, offset, fdt_sz;
+
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }
- blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
+ blob_sz = fdt_sz + 4096;
blob = malloc(blob_sz);
ut_assertnonnull(blob);
@@ -67,10 +74,15 @@ static int
dm_test_fdtdec_add_reserved_memory(struct unit_test_state *uts)
fdt_size_t size;
void *blob;
unsigned long flags = FDTDEC_RESERVED_MEMORY_NO_MAP;
- int blob_sz, parent, subnode;
+ int blob_sz, parent, subnode, fdt_sz;
uint32_t phandle, phandle1;
- blob_sz = fdt_totalsize(gd->fdt_blob) + 128;
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }
+
+ blob_sz = fdt_sz + 128;
blob = malloc(blob_sz);
ut_assertnonnull(blob);
@@ -138,14 +150,19 @@ static int dm_test_fdt_chosen_smbios(struct
unit_test_state *uts)
void *blob;
ulong val;
struct smbios3_entry *entry;
- int chosen, blob_sz;
+ int chosen, blob_sz, fdt_sz;
const fdt64_t *prop;
if (!CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE)) {
return -EAGAIN;
}
- blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }
+
+ blob_sz = fdt_sz + 4096;
blob = memalign(8, blob_sz);
ut_assertnonnull(blob);
On Mon, Dec 8, 2025 at 9:38 PM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest Coverity scan report. I think the test/dm/clk_ccf.c
> report is just a "works as intended" but I'm not sure off-hand about the
> fdtdec.c test. Might be the case the previous test in the file also has
> this problem, and since it's just test code, might also be fine enough.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Dec 8, 2025 at 1:23 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot*
> found with Coverity Scan.
>
> - *New Defects Found:* 2
> - 1 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 2 of 2 defect(s)
>
> Defect Details
>
> ** CID 639831: (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 639831: (TAINTED_SCALAR)
> /test/dm/fdtdec.c: 153 in dm_test_fdt_chosen_smbios()
> 147
> 148 blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149 blob = memalign(8, blob_sz);
> 150 ut_assertnonnull(blob);
> 151
> 152 /* Make a writable copy of the fdt blob */
> >>> CID 639831: (TAINTED_SCALAR)
> >>> Passing tainted expression "gd->fdt_blob->totalsize" to "fdt_open_into", which uses it as an offset.
> 153 ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155 /* Mock SMBIOS table */
> 156 entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157 memcpy(entry->anchor, "_SM3_", 5);
> 158 entry->length = sizeof(struct smbios3_entry);
> /test/dm/fdtdec.c: 153 in dm_test_fdt_chosen_smbios()
> 147
> 148 blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149 blob = memalign(8, blob_sz);
> 150 ut_assertnonnull(blob);
> 151
> 152 /* Make a writable copy of the fdt blob */
> >>> CID 639831: (TAINTED_SCALAR)
> >>> Passing tainted expression "gd->fdt_blob->size_dt_strings" to "fdt_open_into", which uses it as an offset.
> 153 ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155 /* Mock SMBIOS table */
> 156 entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157 memcpy(entry->anchor, "_SM3_", 5);
> 158 entry->length = sizeof(struct smbios3_entry);
> /test/dm/fdtdec.c: 153 in dm_test_fdt_chosen_smbios()
> 147
> 148 blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149 blob = memalign(8, blob_sz);
> 150 ut_assertnonnull(blob);
> 151
> 152 /* Make a writable copy of the fdt blob */
> >>> CID 639831: (TAINTED_SCALAR)
> >>> Passing tainted expression "gd->fdt_blob->size_dt_struct" to "fdt_open_into", which uses it as an offset.
> 153 ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155 /* Mock SMBIOS table */
> 156 entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157 memcpy(entry->anchor, "_SM3_", 5);
> 158 entry->length = sizeof(struct smbios3_entry);
>
> ** CID 639830: Integer handling issues (INTEGER_OVERFLOW)
> /test/dm/clk_ccf.c: 68 in dm_test_clk_ccf()
>
>
> _____________________________________________________________________________________________
> *** CID 639830: Integer handling issues (INTEGER_OVERFLOW)
> /test/dm/clk_ccf.c: 68 in dm_test_clk_ccf()
> 62 ut_asserteq(CLK_SET_RATE_NO_REPARENT, clk->flags);
> 63
> 64 rate = clk_get_parent_rate(clk);
> 65 ut_asserteq(rate, 60000000);
> 66
> 67 rate = clk_set_rate(clk, 60000000);
> >>> CID 639830: Integer handling issues (INTEGER_OVERFLOW)
> >>> Expression "_val1", where "rate" is known to be equal to -38, overflows the type of "_val1", which is type "unsigned int".
> 68 ut_asserteq(rate, -ENOSYS);
> 69
> 70 rate = clk_get_rate(clk);
> 71 ut_asserteq(rate, 60000000);
> 72
> 73 ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_PLL3_80M), &pclk);
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2025-12-09 11:06 ` Adriana Nicolae
@ 2025-12-09 14:24 ` Tom Rini
0 siblings, 0 replies; 19+ messages in thread
From: Tom Rini @ 2025-12-09 14:24 UTC (permalink / raw)
To: Adriana Nicolae; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]
On Tue, Dec 09, 2025 at 01:06:28PM +0200, Adriana Nicolae wrote:
> Hello,
>
> A possible fix for fdtdec.c tests would be to validate the fdt size
> before using it.
> All 3 tests in this file are using the same approach, if the previous tests were
> acceptable the newly added one has some identical parts.
>
> If there is a way to test and fix all these 3 errors, I've put some
> changes which
> might be enough for Coverity to assume that the size of the fdt is safe. Let me
> know how I can test it or should I send an email to post it as a
> separate change?
Thanks. A frustrating thing to me with Coverity is I've never seen a way
to test if a change fixes a bug (if you *pay* for Coverity then you can
do many more runs, and also run it on CI test branches and not pollute
your main results I believe, is why). So we need to do it as a regular
patch.
> diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c
> index ea5a494612c..a3c90d38115 100644
> --- a/test/dm/fdtdec.c
> +++ b/test/dm/fdtdec.c
> @@ -14,14 +14,21 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +#define FDTDEC_MAX_SIZE (2 * 1024 * 1024)
> +
> static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts)
> {
> struct fdt_memory resv;
> void *blob;
> const fdt32_t *prop;
> - int blob_sz, len, offset;
> + int blob_sz, len, offset, fdt_sz;
> +
> + fdt_sz = fdt_totalsize(gd->fdt_blob);
> + if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
> + return -EINVAL;
> + }
Since these are tests, can we ut_assertsomething here instead? Thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2026-04-06 19:12 Fwd: " Tom Rini
@ 2026-04-07 20:44 ` Raymond Mao
0 siblings, 0 replies; 19+ messages in thread
From: Raymond Mao @ 2026-04-07 20:44 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Kory Maincent, Dan Carpenter, Varadarajan Narayanan,
Bo-Chen Chen, David Lechner, Ilias Apalodimas
Hi Tom,
On Mon, Apr 6, 2026 at 3:12 PM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report, now that I've merged next to master, locally
> at least.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Apr 6, 2026 at 12:40 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot*
> found with Coverity Scan.
>
> - *New Defects Found:* 11
> - 15 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 11 of 11 defect(s)
>
> Defect Details
>
> ** CID 645496: (USE_AFTER_FREE)
> /tools/fwumdata_src/fwumdata.c: 94 in parse_config()
> /tools/fwumdata_src/fwumdata.c: 101 in parse_config()
>
>
> _____________________________________________________________________________________________
> *** CID 645496: (USE_AFTER_FREE)
> /tools/fwumdata_src/fwumdata.c: 94 in parse_config()
> 88 &devname,
> 89 &devices[i].devoff,
> 90 &devices[i].mdata_size,
> 91 &devices[i].erase_size);
> 92
> 93 if (rc < 3) {
> >>> CID 645496: (USE_AFTER_FREE)
> >>> Calling "free" frees pointer "devname" which has already been freed.
> 94 free(devname);
> 95 continue;
> 96 }
> 97
> 98 if (rc < 4)
> 99 devices[i].erase_size = devices[i].mdata_size;
> /tools/fwumdata_src/fwumdata.c: 101 in parse_config()
> 95 continue;
> 96 }
> 97
> 98 if (rc < 4)
> 99 devices[i].erase_size = devices[i].mdata_size;
> 100
> >>> CID 645496: (USE_AFTER_FREE)
> >>> Using freed pointer "devname".
> 101 devices[i].devname = devname;
> 102 i++;
> 103 }
> 104
> 105 free(line);
> 106 fclose(fp);
>
> ** CID 645495: Uninitialized variables (UNINIT)
> /fs/fat/fat.c: 175 in disk_rw()
>
>
> _____________________________________________________________________________________________
> *** CID 645495: Uninitialized variables (UNINIT)
> /fs/fat/fat.c: 175 in disk_rw()
> 169 }
> 170 }
> 171 exit:
> 172 if (block)
> 173 free(block);
> 174
> >>> CID 645495: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "ret".
> 175 return (ret == -1) ? -1 : nr_sect;
> 176 }
> 177
> 178 static int disk_read(__u32 sect, __u32 nr_sect, void *buf)
> 179 {
> 180 return disk_rw(sect, nr_sect, buf, true);
>
> ** CID 645494: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 287 in
> mt6359_get_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645494: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 287 in
> mt6359_get_voltage_sel()
> 281
> 282 selector = pmic_reg_read(dev->parent, info->desc.vsel_reg);
> 283 if (selector < 0)
> 284 return selector;
> 285
> 286 selector &= info->desc.vsel_mask;
> >>> CID 645494: Integer handling issues (BAD_SHIFT)
> >>> In expression "selector >>= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 287 selector >>= ffs(info->desc.vsel_mask) - 1;
> 288
> 289 return selector;
> 290 }
> 291
> 292 static int mt6359p_vemc_get_voltage_sel(struct udevice *dev,
> struct mt6359_regulator_info *info)
>
> ** CID 645493: Control flow issues (DEADCODE)
> /drivers/firmware/scmi/pinctrl.c: 206 in
> scmi_pinctrl_settings_get_one()
>
>
> _____________________________________________________________________________________________
> *** CID 645493: Control flow issues (DEADCODE)
> /drivers/firmware/scmi/pinctrl.c: 206 in
> scmi_pinctrl_settings_get_one()
> 200
> 201 msg.out_msg = (u8 *)out;
> 202 msg.out_msg_sz = out_sz;
> 203 in.id = selector;
> 204 in.attr = 0;
> 205 if (config_type == SCMI_PINCTRL_CONFIG_SETTINGS_FUNCTION)
> >>> CID 645493: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "in.attr" inside this statement: "in.attr = ({
> ({
> do {...".
> 206 in.attr = FIELD_PREP(GENMASK(19, 18), 2);
> 207 in.attr |= FIELD_PREP(GENMASK(17, 16), select_type);
> 208 if (config_type != SCMI_PINCTRL_CONFIG_SETTINGS_FUNCTION)
> 209 in.attr |= FIELD_PREP(GENMASK(7, 0), config_type);
> 210
> 211 ret = devm_scmi_process_msg(dev, &msg);
>
> ** CID 645492: (BUFFER_SIZE)
> /drivers/fwu-mdata/raw_mtd.c: 173 in get_fwu_mdata_dev()
> /drivers/fwu-mdata/raw_mtd.c: 183 in get_fwu_mdata_dev()
>
>
> _____________________________________________________________________________________________
> *** CID 645492: (BUFFER_SIZE)
> /drivers/fwu-mdata/raw_mtd.c: 173 in get_fwu_mdata_dev()
> 167 }
> 168
> 169 /* Get the offset of primary and secondary mdata */
> 170 ret = ofnode_read_string_index(dev_ofnode(dev),
> "mdata-parts", 0, &label);
> 171 if (ret)
> 172 return ret;
> >>> CID 645492: (BUFFER_SIZE)
> >>> Calling "strncpy" with a maximum size argument of 50 bytes on destination array "mtd_priv->pri_label" of size 50 bytes might leave the destination string unterminated.
> 173 strncpy(mtd_priv->pri_label, label, 50);
> 174
> 175 ret = flash_partition_offset(mtd_dev, mtd_priv->pri_label, &offset);
> 176 if (ret <= 0)
> 177 return ret;
> 178 mtd_priv->pri_offset = offset;
> /drivers/fwu-mdata/raw_mtd.c: 183 in get_fwu_mdata_dev()
> 177 return ret;
> 178 mtd_priv->pri_offset = offset;
> 179
> 180 ret = ofnode_read_string_index(dev_ofnode(dev),
> "mdata-parts", 1, &label);
> 181 if (ret)
> 182 return ret;
> >>> CID 645492: (BUFFER_SIZE)
> >>> Calling "strncpy" with a maximum size argument of 50 bytes on destination array "mtd_priv->sec_label" of size 50 bytes might leave the destination string unterminated.
> 183 strncpy(mtd_priv->sec_label, label, 50);
> 184
> 185 ret = flash_partition_offset(mtd_dev, mtd_priv->sec_label, &offset);
> 186 if (ret <= 0)
> 187 return ret;
> 188 mtd_priv->sec_offset = offset;
>
> ** CID 645491: Security best practices violations (STRING_OVERFLOW)
> /drivers/fwu-mdata/raw_mtd.c: 244 in fwu_mtd_image_info_populate()
>
>
> _____________________________________________________________________________________________
> *** CID 645491: Security best practices violations (STRING_OVERFLOW)
> /drivers/fwu-mdata/raw_mtd.c: 244 in fwu_mtd_image_info_populate()
> 238 ofnode_read_u32(image, "size", &image_size);
> 239
> 240 mtd_images[off_img].start = bank_offset + image_offset;
> 241 mtd_images[off_img].size = image_size;
> 242 mtd_images[off_img].bank_num = bank_num;
> 243 mtd_images[off_img].image_num = image_num;
> >>> CID 645491: Security best practices violations (STRING_OVERFLOW)
> >>> You might overrun the 37-character fixed-size string "mtd_images[off_img].uuidbuf" by copying "uuid" without checking the length.
> 244 strcpy(mtd_images[off_img].uuidbuf, uuid);
> 245 log_debug("\tImage%d: %s @0x%x\n\n",
> 246 image_num, uuid, bank_offset + image_offset);
> 247 off_img++;
> 248 }
> 249 }
>
> ** CID 645490: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 245 in
> mt6359p_vemc_set_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645490: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 245 in
> mt6359p_vemc_set_voltage_sel()
> 239
> 240 static int mt6359p_vemc_set_voltage_sel(struct udevice *dev,
> 241 struct mt6359_regulator_info *info, unsigned int sel)
> 242 {
> 243 int ret;
> 244
> >>> CID 645490: Integer handling issues (BAD_SHIFT)
> >>> In expression "sel <<= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 245 sel <<= ffs(info->desc.vsel_mask) - 1;
> 246 ret = pmic_reg_write(dev->parent, MT6359P_TMA_KEY_ADDR,
> MT6359P_TMA_KEY);
> 247 if (ret)
> 248 return ret;
> 249
> 250 ret = pmic_reg_read(dev->parent, MT6359P_VM_MODE_ADDR);
>
> ** CID 645489: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 234 in
> mt6359_set_voltage_sel_regmap()
>
>
> _____________________________________________________________________________________________
> *** CID 645489: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 234 in
> mt6359_set_voltage_sel_regmap()
> 228 };
> 229
> 230 static int mt6359_set_voltage_sel_regmap(struct udevice *dev,
> 231 struct mt6359_regulator_info *info,
> 232 unsigned int sel)
> 233 {
> >>> CID 645489: Integer handling issues (BAD_SHIFT)
> >>> In expression "sel <<= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 234 sel <<= ffs(info->desc.vsel_mask) - 1;
> 235
> 236 return pmic_clrsetbits(dev->parent, info->desc.vsel_reg,
> 237 info->desc.vsel_mask, sel);
> 238 }
> 239
>
> ** CID 645488: Error handling issues (CHECKED_RETURN)
> /tools/fwumdata_src/fwumdata.c: 189 in read_device()
>
>
> _____________________________________________________________________________________________
> *** CID 645488: Error handling issues (CHECKED_RETURN)
> /tools/fwumdata_src/fwumdata.c: 189 in read_device()
> 183 {
> 184 if (lseek(dev->fd, dev->devoff, SEEK_SET) < 0) {
> 185 fprintf(stderr, "Seek failed: %s\n", strerror(errno));
> 186 return -errno;
> 187 }
> 188
> >>> CID 645488: Error handling issues (CHECKED_RETURN)
> >>> "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
> 189 if (read(dev->fd, buf, count) < 0) {
> 190 fprintf(stderr, "Read failed: %s\n", strerror(errno));
> 191 return -errno;
> 192 }
> 193
> 194 return 0;
>
> ** CID 645487: Insecure data handling (TAINTED_SCALAR)
> /lib/smbios.c: 1099 in smbios_write_type9_1slot()
>
>
> _____________________________________________________________________________________________
> *** CID 645487: Insecure data handling (TAINTED_SCALAR)
> /lib/smbios.c: 1099 in smbios_write_type9_1slot()
> 1093 * TODO:
> 1094 * peer_groups = <peer_grouping_count> * SMBIOS_TYPE9_PGROUP_SIZE
> 1095 */
> 1096 len += pgroups_size;
> 1097
> 1098 t = map_sysmem(*current, len);
> >>> CID 645487: Insecure data handling (TAINTED_SCALAR)
> >>> Passing tainted expression "len" to "memset", which uses it as an offset. [Note: The source code implementation of the function has been overridden by a builtin model.]
Fixed with patch at:
https://lore.kernel.org/u-boot/20260407204113.3102785-1-raymondmaoca@gmail.com/T/#u
Regards,
Raymond
> 1099 memset(t, 0, len);
> 1100
> 1101 fill_smbios_header(t, SMBIOS_SYSTEM_SLOTS, len, handle);
> 1102
> 1103 /* eos is at the end of the structure */
> 1104 eos_addr = (u8 *)t + len - sizeof(t->eos);
>
> ** CID 645486: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 312 in
> mt6359p_vemc_get_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645486: Integer handling issues (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 312 in
> mt6359p_vemc_get_voltage_sel()
> 306 return -EINVAL;
> 307 }
> 308 if (selector < 0)
> 309 return selector;
> 310
> 311 selector &= info->desc.vsel_mask;
> >>> CID 645486: Integer handling issues (BAD_SHIFT)
> >>> In expression "selector >>= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 312 selector >>= ffs(info->desc.vsel_mask) - 1;
> 313
> 314 return selector;
> 315 }
> 316
> 317 static int mt6359_get_enable(struct udevice *dev)
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New Defects reported by Coverity Scan for Das U-Boot
2026-04-28 14:04 Fwd: " Tom Rini
@ 2026-05-01 22:50 ` Raymond Mao
0 siblings, 0 replies; 19+ messages in thread
From: Raymond Mao @ 2026-05-01 22:50 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot, Ilias Apalodimas, Michal Simek, Christian Pötzsch,
Marek Vasut
Hi Tom,
On Tue, Apr 28, 2026 at 10:04 AM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Apr 27, 2026, 5:41 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to *Das U-Boot*
> found with Coverity Scan.
>
> - *New Defects Found:* 9
> - 77 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> - *Defects Shown:* Showing 9 of 9 defect(s)
>
> Defect Details
>
>
> ** CID 645841: Incorrect expression (BAD_COMPARE)
> /common/bloblist.c: 303 in bloblist_apply_blobs()
>
>
> _____________________________________________________________________________________________
> *** CID 645841: Incorrect expression (BAD_COMPARE)
> /common/bloblist.c: 303 in bloblist_apply_blobs()
> 297 log_err("Failed to apply blob with tag %d\n",
> 298 tag);
> 299 return ret;
> 300 }
> 301
> 302 rec = rec_from_blob(blob - dat_off);
> >>> CID 645841: Incorrect expression (BAD_COMPARE)
> >>> Comparing pointer "rec" against "NULL" using anything besides "==" or "!=" is likely to be incorrect.
> 303 if (rec <= 0) {
> 304 log_err("Blob corrupted\n");
> 305 return -ENOENT;
> 306 }
> 307
> 308 /* Mark applied blob record as void */
>
> ** CID 645840: Uninitialized variables (UNINIT)
>
>
> _____________________________________________________________________________________________
> *** CID 645840: Uninitialized variables (UNINIT)
> /lib/efi_loader/efi_device_path.c: 967 in efi_dp_from_http()
> 961 if ((server && strlen("http://") + strlen(server) + 1 >
> sizeof(tmp)) ||
> 962 (!server && IS_ENABLED(CONFIG_NET_LWIP)))
> 963 return NULL;
> 964
> 965 efi_net_get_addr(&ip, &mask, NULL, dev);
> 966
> >>> CID 645840: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "ip" when calling "efi_dp_from_ipv4".
> 967 dp1 = efi_dp_from_ipv4(&ip, &mask, NULL, dev);
> 968 if (!dp1)
> 969 return NULL;
> 970
> 971
> 972 strcpy(tmp, "http://");
>
> ** CID 645839: Error handling issues (CHECKED_RETURN)
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
>
>
> _____________________________________________________________________________________________
> *** CID 645839: Error handling issues (CHECKED_RETURN)
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645839: Error handling issues (CHECKED_RETURN)
> >>> Calling "fdt_pack" without checking return value (as is done elsewhere 5 out of 6 times).
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
>
> ** CID 645838: Uninitialized variables (UNINIT)
>
>
> _____________________________________________________________________________________________
> *** CID 645838: Uninitialized variables (UNINIT)
> /lib/efi_loader/efi_device_path.c: 967 in efi_dp_from_http()
> 961 if ((server && strlen("http://") + strlen(server) + 1 >
> sizeof(tmp)) ||
> 962 (!server && IS_ENABLED(CONFIG_NET_LWIP)))
> 963 return NULL;
> 964
> 965 efi_net_get_addr(&ip, &mask, NULL, dev);
> 966
> >>> CID 645838: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "mask" when calling "efi_dp_from_ipv4".
> 967 dp1 = efi_dp_from_ipv4(&ip, &mask, NULL, dev);
> 968 if (!dp1)
> 969 return NULL;
> 970
> 971
> 972 strcpy(tmp, "http://");
>
> ** CID 645837: (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
Fix was posted at:
https://lore.kernel.org/u-boot/20260501222801.1596650-1-raymondmaoca@gmail.com/T/#u
Raymond
> *** CID 645837: (TAINTED_SCALAR)
> /lib/fdtdec.c: 1775 in fdtdec_apply_bloblist_dtos()
> 1769 /* Resize if the current space is not sufficient */
> 1770 if (blob_size < padded_size) {
> 1771 ret = bloblist_resize(BLOBLISTT_CONTROL_FDT, padded_size);
> 1772 if (ret)
> 1773 return ret;
> 1774
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_struct" to "fdt_open_into", which uses it as an offset.
> 1775 ret = fdt_open_into(live_fdt, live_fdt, padded_size);
> 1776 if (ret)
> 1777 return ret;
> 1778 }
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_struct" to "fdt_pack", which uses it as an offset.
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> /lib/fdtdec.c: 1775 in fdtdec_apply_bloblist_dtos()
> 1769 /* Resize if the current space is not sufficient */
> 1770 if (blob_size < padded_size) {
> 1771 ret = bloblist_resize(BLOBLISTT_CONTROL_FDT, padded_size);
> 1772 if (ret)
> 1773 return ret;
> 1774
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->totalsize" to "fdt_open_into", which uses it as an offset.
> 1775 ret = fdt_open_into(live_fdt, live_fdt, padded_size);
> 1776 if (ret)
> 1777 return ret;
> 1778 }
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
> /lib/fdtdec.c: 1775 in fdtdec_apply_bloblist_dtos()
> 1769 /* Resize if the current space is not sufficient */
> 1770 if (blob_size < padded_size) {
> 1771 ret = bloblist_resize(BLOBLISTT_CONTROL_FDT, padded_size);
> 1772 if (ret)
> 1773 return ret;
> 1774
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_strings" to "fdt_open_into", which uses it as an offset.
> 1775 ret = fdt_open_into(live_fdt, live_fdt, padded_size);
> 1776 if (ret)
> 1777 return ret;
> 1778 }
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_struct" to "fdt_pack", which uses it as an offset.
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_strings" to "fdt_pack", which uses it as an offset.
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_struct" to "fdt_pack", which uses it as an offset.
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> /lib/fdtdec.c: 1785 in fdtdec_apply_bloblist_dtos()
> 1779
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "live_fdt->size_dt_strings" to "fdt_pack", which uses it as an offset.
> 1785 fdt_pack(live_fdt);
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> /lib/fdtdec.c: 1786 in fdtdec_apply_bloblist_dtos()
> 1780 ret = bloblist_apply_blobs(BLOBLISTT_FDT_OVERLAY,
> fdtdec_apply_dto_blob);
> 1781 if (ret)
> 1782 return ret;
> 1783
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
> 1787 }
> 1788
> 1789 int fdtdec_setup(void)
> 1790 {
> 1791 int ret = -ENOENT;
>
> ** CID 645836: Resource leaks (RESOURCE_LEAK)
> /drivers/gpio/gpio_scmi.c: 201 in scmi_gpio_probe()
>
>
> _____________________________________________________________________________________________
> *** CID 645836: Resource leaks (RESOURCE_LEAK)
> /drivers/gpio/gpio_scmi.c: 201 in scmi_gpio_probe()
> 195 return -ENOMEM;
> 196
> 197 range->base = args.args[0];
> 198 if (range->base != priv->num_pins) {
> 199 dev_err(dev, "no gaps allowed in between pins %d vs %d\n",
> 200 priv->num_pins, range->base);
> >>> CID 645836: Resource leaks (RESOURCE_LEAK)
> >>> Variable "range" going out of scope leaks the storage it points to.
> 201 return -EINVAL;
> 202 }
> 203 range->offset = args.args[1];
> 204 range->npins = args.args[2];
> 205 priv->num_pins += args.args[2];
> 206 list_add_tail(&range->list, &priv->gpio_ranges);
>
> ** CID 645835: Memory - corruptions (OVERRUN)
> /lib/gunzip.c: 248 in gzwrite()
>
>
> _____________________________________________________________________________________________
> *** CID 645835: Memory - corruptions (OVERRUN)
> /lib/gunzip.c: 248 in gzwrite()
> 242 s.avail_out = szwritebuf - numfilled;
> 243 s.next_out = writebuf + numfilled;
> 244 } else {
> 245 s.avail_out = szwritebuf;
> 246 s.next_out = writebuf;
> 247 }
> >>> CID 645835: Memory - corruptions (OVERRUN)
> >>> Calling "inflate" with "s.next_in" and "s.avail_in" is suspicious because of the very large index, 4294966272. The index may be due to a negative parameter being interpreted as unsigned. [Note: The source code implementation of the function has been overridden by a builtin model.]
> 248 r = inflate(&s, Z_SYNC_FLUSH);
> 249 if ((r != Z_OK) &&
> 250 (r != Z_STREAM_END)) {
> 251 printf("Error: inflate() returned %d\n", r);
> 252 goto out;
> 253 }
>
> ** CID 645834: Control flow issues (NO_EFFECT)
> /drivers/virtio/virtio_blk.c: 184 in virtio_blk_do_req()
>
>
> _____________________________________________________________________________________________
> *** CID 645834: Control flow issues (NO_EFFECT)
> /drivers/virtio/virtio_blk.c: 184 in virtio_blk_do_req()
> 178 */
> 179 while (i < blkcnt) {
> 180 u32 blk_per_sg = min(blkcnt - i, seg_sec_cnt * priv->seg_max);
> 181
> 182 ret = virtio_blk_do_single_req(dev, sector + i, blk_per_sg,
> 183 buffer + i * 512, type);
> >>> CID 645834: Control flow issues (NO_EFFECT)
> >>> This less-than-zero comparison of an unsigned value is never true. "ret < 0UL".
> 184 if (ret < 0)
> 185 return ret;
> 186 i += blk_per_sg;
> 187 }
> 188
> 189 return blkcnt >> priv->blksz_shift;
>
> ** CID 645833: Control flow issues (DEADCODE)
> /drivers/virtio/virtio_blk.c: 185 in virtio_blk_do_req()
>
>
> _____________________________________________________________________________________________
> *** CID 645833: Control flow issues (DEADCODE)
> /drivers/virtio/virtio_blk.c: 185 in virtio_blk_do_req()
> 179 while (i < blkcnt) {
> 180 u32 blk_per_sg = min(blkcnt - i, seg_sec_cnt * priv->seg_max);
> 181
> 182 ret = virtio_blk_do_single_req(dev, sector + i, blk_per_sg,
> 183 buffer + i * 512, type);
> 184 if (ret < 0)
> >>> CID 645833: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "return ret;".
> 185 return ret;
> 186 i += blk_per_sg;
> 187 }
> 188
> 189 return blkcnt >> priv->blksz_shift;
> 190 }
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-05-01 22:50 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-31 13:55 Fwd: New Defects reported by Coverity Scan for Das U-Boot Tom Rini
2025-01-01 10:50 ` Abbarapu, Venkatesh
2025-01-02 16:59 ` Tom Rini
-- strict thread matches above, loose matches on Subject: below --
2026-04-28 14:04 Fwd: " Tom Rini
2026-05-01 22:50 ` Raymond Mao
2026-04-06 19:12 Fwd: " Tom Rini
2026-04-07 20:44 ` Raymond Mao
2025-12-08 19:38 Fwd: " Tom Rini
2025-12-09 11:06 ` Adriana Nicolae
2025-12-09 14:24 ` Tom Rini
2025-11-10 18:55 Fwd: " Tom Rini
2025-11-12 8:53 ` Kory Maincent
2025-08-06 18:35 Fwd: " Tom Rini
2025-08-07 1:50 ` Maniyam, Dinesh
2025-07-08 14:10 Fwd: " Tom Rini
2025-07-09 9:13 ` Sughosh Ganu
2025-02-10 22:26 Fwd: " Tom Rini
2025-02-11 22:24 ` Raymond Mao
2025-02-11 22:30 ` Tom Rini
2024-10-16 3:47 Fwd: " Tom Rini
2024-10-16 6:12 ` Ilias Apalodimas
2024-10-16 8:20 ` Abbarapu, Venkatesh
2024-10-16 15:23 ` Raymond Mao
2024-10-07 17:15 Fwd: " Tom Rini
2024-10-07 18:17 ` Richard Weinberger
2024-10-07 20:01 ` Tom Rini
2024-04-22 21:48 Fwd: " Tom Rini
2024-04-23 6:19 ` Ilias Apalodimas
[not found] <65a933ab652b3_da12cbd3e77f998728e5@prd-scan-dashboard-0.mail>
2024-01-19 8:47 ` Fwd: " Heinrich Schuchardt
2024-01-22 6:44 ` Masahisa Kojima
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.