[tom.rini at gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
Ilias Apalodimas
ilias.apalodimas at linaro.org
Tue Nov 7 12:18:19 CET 2023
Hi Tom,
Thanks for the report.
Eddie, can you please check the TCG related ones?
Thanks
/Ilias
On Mon, 6 Nov 2023 at 22:27, Tom Rini <trini at konsulko.com> wrote:
>
> Hey all,
>
> Here's the latest report. I _think_ I passed the right options to
> get_maintainer.pl such that it would only look far enough back in git to
> find the likely authors (along with listed maintainers of the files).
>
> ---------- Forwarded message ---------
> From: <scan-admin at coverity.com>
> Date: Mon, Nov 6, 2023 at 2:58 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini at gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 5 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 13 of 13 defect(s)
>
>
> ** CID 467411: Memory - corruptions (OVERRUN)
>
>
> ________________________________________________________________________________________________________
> *** CID 467411: Memory - corruptions (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 1395 in efi_tcg2_measure_efi_app_invocation()
> 1389
> 1390 ret = tcg2_measure_gpt_data(dev, handle);
> 1391 if (ret != EFI_SUCCESS)
> 1392 goto out;
> 1393
> 1394 for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
> >>> CID 467411: Memory - corruptions (OVERRUN)
> >>> Overrunning buffer pointed to by "(u8 *)&event" of 4 bytes by passing it to a function which accesses it at byte offset 63.
> 1395 ret = measure_event(dev, pcr_index, EV_SEPARATOR,
> 1396 sizeof(event), (u8 *)&event);
> 1397 if (ret != EFI_SUCCESS)
> 1398 goto out;
> 1399 }
> 1400
>
> ** CID 467410: (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 467410: (TAINTED_SCALAR)
> /lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
> 1379 (u8 *)EFI_CALLING_EFI_APPLICATION);
> 1380 if (ret != EFI_SUCCESS)
> 1381 goto out;
> 1382
> 1383 entry = (struct smbios_entry *)find_smbios_table();
> 1384 if (entry) {
> >>> CID 467410: (TAINTED_SCALAR)
> >>> Passing tainted expression "entry->struct_table_length" to "tcg2_measure_smbios", which uses it as an offset.
> 1385 ret = tcg2_measure_smbios(dev, entry);
> 1386 if (ret != EFI_SUCCESS)
> 1387 goto out;
> 1388 }
> 1389
> 1390 ret = tcg2_measure_gpt_data(dev, handle);
> /lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
> 1379 (u8 *)EFI_CALLING_EFI_APPLICATION);
> 1380 if (ret != EFI_SUCCESS)
> 1381 goto out;
> 1382
> 1383 entry = (struct smbios_entry *)find_smbios_table();
> 1384 if (entry) {
> >>> CID 467410: (TAINTED_SCALAR)
> >>> Passing tainted expression "entry->struct_count" to "tcg2_measure_smbios", which uses it as a loop boundary.
> 1385 ret = tcg2_measure_smbios(dev, entry);
> 1386 if (ret != EFI_SUCCESS)
> 1387 goto out;
> 1388 }
> 1389
> 1390 ret = tcg2_measure_gpt_data(dev, handle);
>
> ** CID 467409: Uninitialized variables (UNINIT)
>
>
> ________________________________________________________________________________________________________
> *** CID 467409: Uninitialized variables (UNINIT)
> /test/boot/measurement.c: 48 in measure()
> 42 for (i = 0; i < size; ++i) {
> 43 kernel[i] = 0xf0 | (i & 0xf);
> 44 initrd[i] = (i & 0xf0) | 0xf;
> 45 images.ft_addr[i] = i & 0xff;
> 46 }
> 47
> >>> CID 467409: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "images.os.os" when calling "bootm_measure".
> 48 ut_assertok(bootm_measure(&images));
> 49
> 50 free(images.ft_addr);
> 51 free(initrd);
> 52 free(kernel);
> 53
>
> ** CID 467408: Insecure data handling (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 467408: Insecure data handling (TAINTED_SCALAR)
> /boot/bootm.c: 826 in do_bootm_states()
> 820 env_set_hex("initrd_end", images->initrd_end);
> 821 }
> 822 }
> 823 #endif
> 824 #if CONFIG_IS_ENABLED(OF_LIBFDT) && defined(CONFIG_LMB)
> 825 if (!ret && (states & BOOTM_STATE_FDT)) {
> >>> CID 467408: Insecure data handling (TAINTED_SCALAR)
> >>> Passing tainted expression "*images->ft_addr" to "boot_fdt_add_mem_rsv_regions", which uses it as an offset.
> 826 boot_fdt_add_mem_rsv_regions(&images->lmb,
> images->ft_addr);
> 827 ret = boot_relocate_fdt(&images->lmb, &images->ft_addr,
> 828 &images->ft_len);
> 829 }
> 830 #endif
> 831
>
> ** CID 467407: Uninitialized variables (UNINIT)
> /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
>
>
> ________________________________________________________________________________________________________
> *** CID 467407: Uninitialized variables (UNINIT)
> /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
> 606
> 607 bdesc = dev_get_uclass_plat(bdev);
> 608 bdesc->target = id;
> 609 bdesc->lun = lun;
> 610 bdesc->removable = bd.removable;
> 611 bdesc->type = bd.type;
> >>> CID 467407: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "bd.bb".
> 612 bdesc->bb = bd.bb;
> 613 memcpy(&bdesc->vendor, &bd.vendor, sizeof(bd.vendor));
> 614 memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
> 615 memcpy(&bdesc->revision, &bd.revision, sizeof(bd.revision));
> 616 if (IS_ENABLED(CONFIG_SYS_BIG_ENDIAN)) {
> 617 ata_swap_buf_le16((u16 *)&bdesc->vendor,
> sizeof(bd.vendor) / 2);
>
> ** CID 467406: Memory - corruptions (OVERRUN)
>
>
> ________________________________________________________________________________________________________
> *** CID 467406: Memory - corruptions (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 885 in efi_append_scrtm_version()
> 879 * @Return: status code
> 880 */
> 881 static efi_status_t efi_append_scrtm_version(struct udevice *dev)
> 882 {
> 883 efi_status_t ret;
> 884
> >>> CID 467406: Memory - corruptions (OVERRUN)
> >>> Overrunning array "version_string" of 50 bytes by passing it to a function which accesses it at byte offset 63.
> 885 ret = measure_event(dev, 0, EV_S_CRTM_VERSION,
> 886 strlen(version_string) + 1, (u8
> *)version_string);
> 887
> 888 return ret;
> 889 }
> 890
>
> ** CID 467405: Memory - illegal accesses (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()
>
>
> ________________________________________________________________________________________________________
> *** CID 467405: Memory - illegal accesses (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()
> 656 if (domain_id > ARRAY_SIZE(scmi_pwdom)) {
> 657 out->status = SCMI_NOT_FOUND;
> 658
> 659 return 0;
> 660 }
> 661
> >>> CID 467405: Memory - illegal accesses (OVERRUN)
> >>> Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "domain_id" (which evaluates to 3).
> 662 out->pstate = scmi_pwdom[domain_id].pstate;
> 663 out->status = SCMI_SUCCESS;
> 664
> 665 return 0;
> 666 }
> 667
>
> ** CID 467404: Control flow issues (DEADCODE)
> /test/cmd/mbr.c: 217 in build_mbr_parts()
>
>
> ________________________________________________________________________________________________________
> *** CID 467404: Control flow issues (DEADCODE)
> /test/cmd/mbr.c: 217 in build_mbr_parts()
> 211 return 1;
> 212 strcat(cur_buf, mbr_parts_p5);
> 213 bytes_remaining -= cur_str_size;
> 214
> 215 }
> 216 else if (num_parts > 5)
> >>> CID 467404: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "return 1U;".
> 217 return 1;
> 218 }
> 219 }
> 220 }
> 221
> 222 cur_str_size = sizeof(mbr_parts_tail);
>
> ** CID 467403: Error handling issues (CHECKED_RETURN)
> /test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()
>
>
> ________________________________________________________________________________________________________
> *** CID 467403: Error handling issues (CHECKED_RETURN)
> /test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()
> 863 node = ofnode_path("/usb at 2");
> 864
> 865 ut_assert(!ofnode_is_enabled(node));
> 866 ut_assertok(ofnode_set_enabled(node, true));
> 867 ut_asserteq(true, ofnode_is_enabled(node));
> 868
> >>> CID 467403: Error handling issues (CHECKED_RETURN)
> >>> Calling "device_bind_driver_to_node" without checking return value (as is done elsewhere 12 out of 15 times).
> 869 device_bind_driver_to_node(dm_root(), "usb_sandbox",
> "usb at 2", node,
> 870 &dev);
> 871 ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev));
> 872
> 873 /* Test string property setting */
> 874 ut_assert(device_is_compatible(dev, "sandbox,usb"));
>
> ** CID 467402: (CHECKED_RETURN)
> /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> /drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
>
>
> ________________________________________________________________________________________________________
> *** CID 467402: (CHECKED_RETURN)
> /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> 731
> 732 debug("starting_lba : %llu\n",
> le64_to_cpu(plat->gpt_e->starting_lba));
> 733 debug("ending_lba : %llu\n",
> le64_to_cpu(plat->gpt_e->ending_lba));
> 734
> 735 memcpy(plat->gpt_e->partition_type_guid.b,
> &partition_basic_data_guid, 16);
> 736
> >>> CID 467402: (CHECKED_RETURN)
> >>> Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 737 uuid_str_to_bin(plat->uuid_part_str,
> plat->gpt_e->unique_partition_guid.b,
> 738 UUID_STR_FORMAT_GUID);
> 739
> 740 efiname_len = sizeof(plat->gpt_e->partition_name) /
> sizeof(efi_char16_t);
> 741 dosname_len = sizeof(name);
> 742
> /drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
> 749 plat->gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
> 750 plat->gpt_h->first_usable_lba = cpu_to_le64(64);
> 751 plat->gpt_h->last_usable_lba = cpu_to_le64(LBA - 34);
> 752 plat->gpt_h->num_partition_entries = cpu_to_le32(1);
> 753 plat->gpt_h->sizeof_partition_entry =
> cpu_to_le32(sizeof(gpt_entry));
> 754
> >>> CID 467402: (CHECKED_RETURN)
> >>> Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 755 uuid_str_to_bin(plat->uuid_disk_str, plat->gpt_h->disk_guid.b,
> 756 UUID_STR_FORMAT_GUID);
> 757
> 758 plat->gpt_h->partition_entry_array_crc32 = 0;
> 759 calc_crc32 = efi_crc32((const unsigned char *)plat->gpt_e,
> 760
> le32_to_cpu(plat->gpt_h->num_partition_entries) *
>
> ** CID 467401: Memory - corruptions (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
>
>
> ________________________________________________________________________________________________________
> *** CID 467401: Memory - corruptions (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
> 623 (in->pstate != SCMI_PWD_PSTATE_TYPE_LOST && in->pstate)) {
> 624 *status = SCMI_INVALID_PARAMETERS;
> 625
> 626 return 0;
> 627 }
> 628
> >>> CID 467401: Memory - corruptions (OVERRUN)
> >>> Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "in->domain_id" (which evaluates to 3).
> 629 scmi_pwdom[in->domain_id].pstate = in->pstate;
> 630 *status = SCMI_SUCCESS;
> 631
> 632 return 0;
> 633 }
> 634
>
> ** CID 467400: Memory - illegal accesses (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()
>
>
> ________________________________________________________________________________________________________
> *** CID 467400: Memory - illegal accesses (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()
> 992 guidcpy(&event->variable_name, guid);
> 993 event->unicode_name_length = u16_strlen(var_name);
> 994 event->variable_data_length = data_size;
> 995 memcpy(event->unicode_name, var_name,
> 996 (event->unicode_name_length * sizeof(u16)));
> 997 if (data) {
> >>> CID 467400: Memory - illegal accesses (OVERRUN)
> >>> Overrunning array of 2 bytes at byte offset 2 by dereferencing pointer "(u16 *)event->unicode_name + event->unicode_name_length". [Note: The source code implementation of the function has been overridden by a builtin model.]
> 998 memcpy((u16 *)event->unicode_name +
> event->unicode_name_length,
> 999 data, data_size);
> 1000 }
> 1001 ret = measure_event(dev, pcr_index, event_type, event_size,
> 1002 (u8 *)event);
> 1003 free(event);
>
> ** CID 467399: Code maintainability issues (UNUSED_VALUE)
> /lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()
>
>
> ________________________________________________________________________________________________________
> *** CID 467399: Code maintainability issues (UNUSED_VALUE)
> /lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()
> 942
> 943 /*
> 944 * Add SCRTM version to the log if previous firmmware
> 945 * doesn't pass an eventlog.
> 946 */
> 947 if (!elog.found)
> >>> CID 467399: Code maintainability issues (UNUSED_VALUE)
> >>> Assigning value from "efi_append_scrtm_version(dev)" to "ret" here, but that stored value is overwritten before it can be used.
> 948 ret = efi_append_scrtm_version(dev);
> 949
> 950 ret = create_final_event();
> 951 if (ret != EFI_SUCCESS)
> 952 goto free_pool;
> 953
>
>
> --
> Tom
More information about the U-Boot
mailing list