* Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] <611aaf735d268_21438d2b07184e399c79439@prd-scan-dashboard-0.mail>
@ 2021-08-17 5:21 ` Heinrich Schuchardt
2021-08-17 15:17 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2021-08-17 5:21 UTC (permalink / raw)
To: Tom Rini; +Cc: Simon Glass, U-Boot Mailing List
Hello Tom,
I suggest to mark these as invalid:
CID 338485
CID 338490
CID 338489
Best regards
Heinrich
-------- Forwarded Message --------
Subject: New Defects reported by Coverity Scan for Das U-Boot
Date: Mon, 16 Aug 2021 18:33:23 +0000 (UTC)
From: scan-admin@coverity.com
To: xypron.glpk@gmx.de
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot
found with Coverity Scan.
7 new defect(s) introduced to Das U-Boot found with Coverity Scan.
3 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 7 of 7 defect(s)
** CID 338491: Null pointer dereferences (NULL_RETURNS)
/tools/kwbimage.c: 1066 in export_pub_kak_hash()
________________________________________________________________________________________________________
*** CID 338491: Null pointer dereferences (NULL_RETURNS)
/tools/kwbimage.c: 1066 in export_pub_kak_hash()
1060 int res;
1061 1062 hashf = fopen("pub_kak_hash.txt", "w");
1063 1064 res = kwb_export_pubkey(kak, &secure_hdr->kak, hashf,
"KAK");
1065 >>> CID 338491: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "hashf" when calling "fclose".
1066 fclose(hashf);
1067 1068 return res < 0 ? 1 : 0;
1069 }
1070 1071 int kwb_sign_csk_with_kak(struct image_tool_params
*params,
** CID 338490: Control flow issues (DEADCODE)
/drivers/tpm/sandbox_common.c: 34 in sb_tpm_index_to_seq()
________________________________________________________________________________________________________
*** CID 338490: Control flow issues (DEADCODE)
/drivers/tpm/sandbox_common.c: 34 in sb_tpm_index_to_seq()
28 case FWMP_NV_INDEX:
29 return NV_SEQ_FWMP;
30 case MRC_REC_HASH_NV_INDEX:
31 return NV_SEQ_REC_HASH;
32 case 0:
33 return NV_SEQ_GLOBAL_LOCK;
>>> CID 338490: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "case TPM_NV_INDEX_LOCK:".
34 case TPM_NV_INDEX_LOCK:
35 return NV_SEQ_ENABLE_LOCKING;
36 }
37 38 printf("Invalid nv index %#x\n", index);
39 return -1;
** CID 338489: Control flow issues (DEADCODE)
/drivers/tpm/tpm2_tis_sandbox.c: 652 in sandbox_tpm2_xfer()
________________________________________________________________________________________________________
*** CID 338489: Control flow issues (DEADCODE)
/drivers/tpm/tpm2_tis_sandbox.c: 652 in sandbox_tpm2_xfer()
646 647 for (i = 0; i < SANDBOX_TPM_PCR_NB; i++)
648 if (pcr_map & BIT(i))
649 pcr_index = i;
650 651 if (pcr_index >= SANDBOX_TPM_PCR_NB) {
>>> CID 338489: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "printf("Invalid index %d, s...".
652 printf("Invalid index %d, sandbox TPM handles up to %d PCR(s)\n",
653 pcr_index, SANDBOX_TPM_PCR_NB);
654 rc = TPM2_RC_VALUE;
655 return sandbox_tpm2_fill_buf(recv, recv_len, tag, rc);
656 }
657
** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
/tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak()
________________________________________________________________________________________________________
*** CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
/tools/kwbimage.c: 1093 in kwb_sign_csk_with_kak()
1087 if (export_pub_kak_hash(kak, secure_hdr))
1088 return 1;
1089 1090 if (kwb_import_pubkey(&kak_pub, &secure_hdr->kak,
"KAK") < 0)
1091 return 1;
1092 >>> CID 338488: Memory - illegal accesses (NEGATIVE_RETURNS)
>>> Using variable "csk_idx" as an index to array "secure_hdr->csk".
1093 if (kwb_export_pubkey(csk, &secure_hdr->csk[csk_idx], NULL,
"CSK") < 0)
1094 return 1;
1095 1096 if (kwb_sign_and_verify(kak, &secure_hdr->csk,
1097 sizeof(secure_hdr->csk) +
1098 sizeof(secure_hdr->csksig),
** CID 338487: Null pointer dereferences (FORWARD_NULL)
________________________________________________________________________________________________________
*** CID 338487: Null pointer dereferences (FORWARD_NULL)
/test/dm/ecdsa.c: 34 in dm_test_ecdsa_verify()
28 struct image_sign_info info = {
29 .checksum = &algo,
30 };
31 32 ut_assertok(uclass_get(UCLASS_ECDSA, &ucp));
33 ut_assertnonnull(ucp);
>>> CID 338487: Null pointer dereferences (FORWARD_NULL)
>>> Passing "&info" to "ecdsa_verify", which dereferences null "info.fdt_blob".
34 ut_asserteq(-ENODEV, ecdsa_verify(&info, NULL, 0, NULL, 0));
35 36 return 0;
37 }
** CID 338486: Null pointer dereferences (NULL_RETURNS)
/tools/kwbimage.c: 836 in kwb_dump_fuse_cmds()
________________________________________________________________________________________________________
*** CID 338486: Null pointer dereferences (NULL_RETURNS)
/tools/kwbimage.c: 836 in kwb_dump_fuse_cmds()
830 return 0;
831 832 if (!strcmp(e->name, "a38x")) {
833 FILE *out = fopen("kwb_fuses_a38x.txt", "w+");
834 835 kwb_dump_fuse_cmds_38x(out, sec_hdr);
>>> CID 338486: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "out" when calling "fclose".
836 fclose(out);
837 goto done;
838 }
839 840 ret = -ENOSYS;
841
** CID 338485: Security best practices violations (STRING_OVERFLOW)
/test/str_ut.c: 126 in run_strtoull()
________________________________________________________________________________________________________
*** CID 338485: Security best practices violations (STRING_OVERFLOW)
/test/str_ut.c: 126 in run_strtoull()
120 bool upper)
121 {
122 char out[TEST_STR_SIZE];
123 char *endp;
124 unsigned long long val;
125 >>> CID 338485: Security best practices violations
(STRING_OVERFLOW)
>>> You might overrun the 200-character fixed-size string "out" by copying "str" without checking the length.
126 strcpy(out, str);
127 if (upper)
128 str_to_upper(out, out, -1);
129 130 val = simple_strtoull(out, &endp, base);
131 ut_asserteq(expect_val, val);
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3DTOyS_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2W-2Bt1lOhx2R0KW7-2FLGCeFuld7ZlXBjbQfd5e2hCM-2BEvdEIHjMXCW-2B3DQc7pN8d55Py6IHBHtDywdLYofSZLYRoliG1Jt-2F9VcIWcj4wOYgz0KmpTLxnK-2FsIaUz26JI1WnWdPzQvLYFOv1ZWqRBfkRFJkNjWWQNM8drRfC5rWPO160w-3D-3D
To manage Coverity Scan email notifications for "xypron.glpk@gmx.de",
click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXx4Y-2F1WK-2FIlbEOzfoxXLI-2FdwA0wwGn90rGGrBgiHW-2ByLDLbUOEV7XOvtc9zJmj9LPyrT06WSaMnNrm6wfrUN-2BXuWoaHdqOoEyL7CQlGSiE-2BfE-3D9MA4_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2W-2Bt1lOhx2R0KW7-2FLGCeFul9vKV6MPfCXhJ2U2Vsc5BZ82XBwntw1jOvGCwwx08PHX5gHT6KmetbutfLsQSRAcWH5ZjapaXsfz24pAvhFoc7v3IDV6kpXprCynWhxTO-2BIBqoiwb55fqAbRuuYTILM-2Bcb9AXlhwuEBtFbnVou6zHNQ-3D-3D
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2021-08-17 5:21 ` Fwd: New Defects reported by Coverity Scan for Das U-Boot Heinrich Schuchardt
@ 2021-08-17 15:17 ` Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2021-08-17 15:17 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: Simon Glass, U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 204 bytes --]
On Tue, Aug 17, 2021 at 07:21:43AM +0200, Heinrich Schuchardt wrote:
> Hello Tom,
>
> I suggest to mark these as invalid:
>
> CID 338485
> CID 338490
> CID 338489
Done, thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-04-22 21:48 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2024-04-22 21:48 UTC (permalink / raw)
To: u-boot; +Cc: Charles Hardin, Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2024-01-29 23:55 Tom Rini
@ 2024-01-30 8:14 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-01-30 8:14 UTC (permalink / raw)
To: Tom Rini; +Cc: Ilias Apalodimas, u-boot
On 1/30/24 00:55, Tom Rini wrote:
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Jan 29, 2024 at 6:51 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.
>
> 1 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 1 of 1 defect(s)
>
>
> ** CID 480240: Insecure data handling (TAINTED_SCALAR)
> /cmd/efidebug.c: 192 in do_efi_capsule_esrt()
>
>
> ________________________________________________________________________________________________________
> *** CID 480240: Insecure data handling (TAINTED_SCALAR)
> /cmd/efidebug.c: 192 in do_efi_capsule_esrt()
> 186
> 187 printf("========================================\n");
> 188 printf("ESRT: fw_resource_count=%d\n", esrt->fw_resource_count);
> 189 printf("ESRT: fw_resource_count_max=%d\n",
> esrt->fw_resource_count_max);
> 190 printf("ESRT: fw_resource_version=%lld\n",
> esrt->fw_resource_version);
> 191
>>>> CID 480240: Insecure data handling (TAINTED_SCALAR)
>>>> Using tainted variable "esrt->fw_resource_count" as a loop boundary.
> 192 for (int idx = 0; idx < esrt->fw_resource_count; idx++) {
> 193 printf("[entry
> %d]==============================\n", idx);
> 194 printf("ESRT: fw_class=%pUL\n",
> &esrt->entries[idx].fw_class);
> 195 printf("ESRT: fw_type=%s\n",
> EFI_FW_TYPE_STR_GET(esrt->entries[idx].fw_type));
> 196 printf("ESRT: fw_version=%d\n",
> esrt->entries[idx].fw_version);
> 197 printf("ESRT: lowest_supported_fw_version=%d\n",
>
> ----- End forwarded message -----
>
Coverity sees any conversion from void * as a hint to tainted data. The
ESRT might be manipulated by some EFI app but we want to display it. So
I marked this Coverity issue as intentional.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-01-29 23:55 Tom Rini
2024-01-30 8:14 ` Heinrich Schuchardt
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2024-01-29 23:55 UTC (permalink / raw)
To: u-boot, Heinrich Schuchardt
[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]
Here's the latest report.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Jan 29, 2024 at 6:51 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.
1 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 1 of 1 defect(s)
** CID 480240: Insecure data handling (TAINTED_SCALAR)
/cmd/efidebug.c: 192 in do_efi_capsule_esrt()
________________________________________________________________________________________________________
*** CID 480240: Insecure data handling (TAINTED_SCALAR)
/cmd/efidebug.c: 192 in do_efi_capsule_esrt()
186
187 printf("========================================\n");
188 printf("ESRT: fw_resource_count=%d\n", esrt->fw_resource_count);
189 printf("ESRT: fw_resource_count_max=%d\n",
esrt->fw_resource_count_max);
190 printf("ESRT: fw_resource_version=%lld\n",
esrt->fw_resource_version);
191
>>> CID 480240: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "esrt->fw_resource_count" as a loop boundary.
192 for (int idx = 0; idx < esrt->fw_resource_count; idx++) {
193 printf("[entry
%d]==============================\n", idx);
194 printf("ESRT: fw_class=%pUL\n",
&esrt->entries[idx].fw_class);
195 printf("ESRT: fw_type=%s\n",
EFI_FW_TYPE_STR_GET(esrt->entries[idx].fw_type));
196 printf("ESRT: fw_version=%d\n",
esrt->entries[idx].fw_version);
197 printf("ESRT: lowest_supported_fw_version=%d\n",
----- End forwarded message -----
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2024-01-27 20:56 ` Heinrich Schuchardt
@ 2024-01-28 8:51 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-01-28 8:51 UTC (permalink / raw)
To: Tom Rini; +Cc: Ilias Apalodimas, u-boot
On 1/27/24 21:56, Heinrich Schuchardt wrote:
>
>
> Am 27. Januar 2024 16:40:18 MEZ schrieb Tom Rini <trini@konsulko.com>:
>> Hey, I'll just pass this on directly rather than to the list.
>>
>> ---------- Forwarded message ---------
>> From: <scan-admin@coverity.com>
>> Date: Sat, Jan 27, 2024 at 10:36 AM
>> 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.
>>
>> 1 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>>
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 1 of 1 defect(s)
>>
>>
>> ** CID 479279: (TAINTED_SCALAR)
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 479279: (TAINTED_SCALAR)
>> /cmd/smbios.c: 180 in do_smbios()
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180 smbios_print_generic(pos);
>> 181 break;
>> 182 }
>> 183 }
>> 184
>> 185 return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 154 in do_smbios()
>> 148 size = entry2->length;
>> 149 max_struct_size = entry2->max_struct_size;
>> 150 } else {
>> 151 log_err("Unknown SMBIOS anchor format\n");
>> 152 return CMD_RET_FAILURE;
>> 153 }
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>> 154 if (table_compute_checksum(entry, size)) {
>> 155 log_err("Invalid anchor checksum\n");
>> 156 return CMD_RET_FAILURE;
>> 157 }
>> 158 printf("SMBIOS %s present.\n", version);
>> 159
>> /cmd/smbios.c: 174 in do_smbios()
>> 168 (unsigned long long)map_to_sysmem(pos));
>> 169 switch (pos->type) {
>> 170 case 1:
>> 171 smbios_print_type1((struct smbios_type1 *)pos);
>> 172 break;
>> 173 case 2:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>> /cmd/smbios.c: 154 in do_smbios()
>> 148 size = entry2->length;
>> 149 max_struct_size = entry2->max_struct_size;
>> 150 } else {
>> 151 log_err("Unknown SMBIOS anchor format\n");
>> 152 return CMD_RET_FAILURE;
>> 153 }
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>> 154 if (table_compute_checksum(entry, size)) {
>> 155 log_err("Invalid anchor checksum\n");
>> 156 return CMD_RET_FAILURE;
>> 157 }
>> 158 printf("SMBIOS %s present.\n", version);
>> 159
>> /cmd/smbios.c: 180 in do_smbios()
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180 smbios_print_generic(pos);
>> 181 break;
>> 182 }
>> 183 }
>> 184
>> 185 return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168 (unsigned long long)map_to_sysmem(pos));
>> 169 switch (pos->type) {
>> 170 case 1:
>> 171 smbios_print_type1((struct smbios_type1 *)pos);
>> 172 break;
>> 173 case 2:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>> /cmd/smbios.c: 174 in do_smbios()
>> 168 (unsigned long long)map_to_sysmem(pos));
>> 169 switch (pos->type) {
>> 170 case 1:
>> 171 smbios_print_type1((struct smbios_type1 *)pos);
>> 172 break;
>> 173 case 2:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>> /cmd/smbios.c: 180 in do_smbios()
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180 smbios_print_generic(pos);
>> 181 break;
>> 182 }
>> 183 }
>> 184
>> 185 return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 180 in do_smbios()
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180 smbios_print_generic(pos);
>> 181 break;
>> 182 }
>> 183 }
>> 184
>> 185 return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168 (unsigned long long)map_to_sysmem(pos));
>> 169 switch (pos->type) {
>> 170 case 1:
>> 171 smbios_print_type1((struct smbios_type1 *)pos);
>> 172 break;
>> 173 case 2:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>> /cmd/smbios.c: 180 in do_smbios()
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180 smbios_print_generic(pos);
>> 181 break;
>> 182 }
>> 183 }
>> 184
>> 185 return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168 (unsigned long long)map_to_sysmem(pos));
>> 169 switch (pos->type) {
>> 170 case 1:
>> 171 smbios_print_type1((struct smbios_type1 *)pos);
>> 172 break;
>> 173 case 2:
>>>>> CID 479279: (TAINTED_SCALAR)
>>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174 smbios_print_type2((struct smbios_type2 *)pos);
>> 175 break;
>> 176 case 127:
>> 177 smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178 break;
>> 179 default:
>>
>
> The values may come from QEMU, so may be "tainted". We could check the length of the individual structures against the total size of the SMBIOS table.
>
In Coverity I marked this as false positive with the following comment:
"The only case in which the data is tainted is when copying the smbios
table from a prior firmware state when running as EFI app or from QEMU.
Sanity checks should not be in the smbios command but where we import
the table."
Best regards
Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] <20240127154018.GC785631@bill-the-cat>
@ 2024-01-27 20:56 ` Heinrich Schuchardt
2024-01-28 8:51 ` Heinrich Schuchardt
0 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-01-27 20:56 UTC (permalink / raw)
To: Tom Rini; +Cc: Ilias Apalodimas, u-boot
Am 27. Januar 2024 16:40:18 MEZ schrieb Tom Rini <trini@konsulko.com>:
>Hey, I'll just pass this on directly rather than to the list.
>
>---------- Forwarded message ---------
>From: <scan-admin@coverity.com>
>Date: Sat, Jan 27, 2024 at 10:36 AM
>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.
>
>1 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
>New defect(s) Reported-by: Coverity Scan
>Showing 1 of 1 defect(s)
>
>
>** CID 479279: (TAINTED_SCALAR)
>
>
>________________________________________________________________________________________________________
>*** CID 479279: (TAINTED_SCALAR)
>/cmd/smbios.c: 180 in do_smbios()
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>180 smbios_print_generic(pos);
>181 break;
>182 }
>183 }
>184
>185 return CMD_RET_SUCCESS;
>/cmd/smbios.c: 154 in do_smbios()
>148 size = entry2->length;
>149 max_struct_size = entry2->max_struct_size;
>150 } else {
>151 log_err("Unknown SMBIOS anchor format\n");
>152 return CMD_RET_FAILURE;
>153 }
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>154 if (table_compute_checksum(entry, size)) {
>155 log_err("Invalid anchor checksum\n");
>156 return CMD_RET_FAILURE;
>157 }
>158 printf("SMBIOS %s present.\n", version);
>159
>/cmd/smbios.c: 174 in do_smbios()
>168 (unsigned long long)map_to_sysmem(pos));
>169 switch (pos->type) {
>170 case 1:
>171 smbios_print_type1((struct smbios_type1 *)pos);
>172 break;
>173 case 2:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>/cmd/smbios.c: 154 in do_smbios()
>148 size = entry2->length;
>149 max_struct_size = entry2->max_struct_size;
>150 } else {
>151 log_err("Unknown SMBIOS anchor format\n");
>152 return CMD_RET_FAILURE;
>153 }
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>154 if (table_compute_checksum(entry, size)) {
>155 log_err("Invalid anchor checksum\n");
>156 return CMD_RET_FAILURE;
>157 }
>158 printf("SMBIOS %s present.\n", version);
>159
>/cmd/smbios.c: 180 in do_smbios()
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>180 smbios_print_generic(pos);
>181 break;
>182 }
>183 }
>184
>185 return CMD_RET_SUCCESS;
>/cmd/smbios.c: 174 in do_smbios()
>168 (unsigned long long)map_to_sysmem(pos));
>169 switch (pos->type) {
>170 case 1:
>171 smbios_print_type1((struct smbios_type1 *)pos);
>172 break;
>173 case 2:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>/cmd/smbios.c: 174 in do_smbios()
>168 (unsigned long long)map_to_sysmem(pos));
>169 switch (pos->type) {
>170 case 1:
>171 smbios_print_type1((struct smbios_type1 *)pos);
>172 break;
>173 case 2:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>/cmd/smbios.c: 180 in do_smbios()
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>180 smbios_print_generic(pos);
>181 break;
>182 }
>183 }
>184
>185 return CMD_RET_SUCCESS;
>/cmd/smbios.c: 180 in do_smbios()
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>180 smbios_print_generic(pos);
>181 break;
>182 }
>183 }
>184
>185 return CMD_RET_SUCCESS;
>/cmd/smbios.c: 174 in do_smbios()
>168 (unsigned long long)map_to_sysmem(pos));
>169 switch (pos->type) {
>170 case 1:
>171 smbios_print_type1((struct smbios_type1 *)pos);
>172 break;
>173 case 2:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>/cmd/smbios.c: 180 in do_smbios()
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>180 smbios_print_generic(pos);
>181 break;
>182 }
>183 }
>184
>185 return CMD_RET_SUCCESS;
>/cmd/smbios.c: 174 in do_smbios()
>168 (unsigned long long)map_to_sysmem(pos));
>169 switch (pos->type) {
>170 case 1:
>171 smbios_print_type1((struct smbios_type1 *)pos);
>172 break;
>173 case 2:
>>>> CID 479279: (TAINTED_SCALAR)
>>>> Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>174 smbios_print_type2((struct smbios_type2 *)pos);
>175 break;
>176 case 127:
>177 smbios_print_type127((struct
>smbios_type127 *)pos);
>178 break;
>179 default:
>
The values may come from QEMU, so may be "tainted". We could check the length of the individual structures against the total size of the SMBIOS table.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2024-01-22 23:30 Tom Rini
@ 2024-01-23 8:15 ` Hugo Cornelis
0 siblings, 0 replies; 36+ messages in thread
From: Hugo Cornelis @ 2024-01-23 8:15 UTC (permalink / raw)
To: u-boot, Tom Rini; +Cc: Hugo Cornelis
Hi Tom, sorry about that. Please find attached a patch.
Can you please review?
Thanks, Hugo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-01-22 23:52 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2024-01-22 23:52 UTC (permalink / raw)
To: u-boot
[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]
I've now updated to the latest Coverity scan tool and that eliminated
some previous defects and found two new ones:
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Jan 22, 2024 at 6:42 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.
8 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 478862: Memory - corruptions (OVERRUN)
________________________________________________________________________________________________________
*** CID 478862: Memory - corruptions (OVERRUN)
/lib/initcall.c: 82 in initcall_run_list()
76 if (ret) {
77 if (CONFIG_IS_ENABLED(EVENT)) {
78 char buf[60];
79
80 /* don't worry about buf size as we are dying here */
81 if (type) {
>>> CID 478862: Memory - corruptions (OVERRUN)
>>> Overrunning callee's array of size 15 by passing argument "type" (which evaluates to 255) in call to "event_type_name".
82 sprintf(buf, "event %d/%s", type,
83 event_type_name(type));
84 } else {
85 sprintf(buf, "call %p", func);
86 }
87
** CID 478861: Memory - corruptions (OVERRUN)
________________________________________________________________________________________________________
*** CID 478861: Memory - corruptions (OVERRUN)
/cmd/nvedit.c: 356 in print_static_flags()
350 static int print_static_flags(const char *var_name, const char *flags,
351 void *priv)
352 {
353 enum env_flags_vartype type = env_flags_parse_vartype(flags);
354 enum env_flags_varaccess access =
env_flags_parse_varaccess(flags);
355
>>> CID 478861: Memory - corruptions (OVERRUN)
>>> Overrunning callee's array of size 4 by passing argument "access" (which evaluates to 4) in call to "env_flags_get_varaccess_name".
356 printf("\t%-20s %-20s %-20s\n", var_name,
357 env_flags_get_vartype_name(type),
358 env_flags_get_varaccess_name(access));
359
360 return 0;
361 }
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-01-22 23:30 Tom Rini
2024-01-23 8:15 ` Hugo Cornelis
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2024-01-22 23:30 UTC (permalink / raw)
To: u-boot, Hugo Cornelis
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]
Hey all,
Here's the latest Coverity scan report.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Jan 22, 2024 at 6:26 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.
1 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 1 of 1 defect(s)
** CID 478860: Code maintainability issues (UNUSED_VALUE)
/tools/image-host.c: 359 in fit_image_read_key_iv_data()
________________________________________________________________________________________________________
*** CID 478860: Code maintainability issues (UNUSED_VALUE)
/tools/image-host.c: 359 in fit_image_read_key_iv_data()
353 if (ret >= sizeof(filename)) {
354 printf("Can't format the key or IV filename
when setting up the cipher: insufficient buffer space\n");
355 ret = -1;
356 }
357 if (ret < 0) {
358 printf("Can't format the key or IV filename
when setting up the cipher: snprintf error\n");
>>> CID 478860: Code maintainability issues (UNUSED_VALUE)
>>> Assigning value "-1" to "ret" here, but that stored value is overwritten before it can be used.
359 ret = -1;
360 }
361
362 ret = fit_image_read_data(filename, key_iv_data, expected_size);
363
364 return ret;
----- End forwarded message -----
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] <65a933ab652b3_da12cbd3e77f998728e5@prd-scan-dashboard-0.mail>
@ 2024-01-19 8:47 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-01-19 8:47 UTC (permalink / raw)
To: Masahisa Kojima; +Cc: U-Boot Mailing List, Ilias Apalodimas
________________________________________________________________________________________________________
*** 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?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-01-18 14:35 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2024-01-18 14:35 UTC (permalink / raw)
To: u-boot, Ilias Apalodimas, Heinrich Schuchardt
[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]
Here's the current set of new issues since I last ran Coverity.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Thu, Jan 18, 2024 at 9:20 AM
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.
16 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 478334: Memory - corruptions (OVERRUN)
________________________________________________________________________________________________________
*** CID 478334: Memory - corruptions (OVERRUN)
/cmd/eficonfig.c: 534 in eficonfig_create_device_path()
528 p += fp_size;
529 *((struct efi_device_path *)p) = END;
530
531 dp = efi_dp_shorten(dp_volume);
532 if (!dp)
533 dp = dp_volume;
>>> CID 478334: Memory - corruptions (OVERRUN)
>>> Overrunning struct type efi_device_path of 4 bytes by passing it to a function which accesses it at byte offset 5 using argument "fp->dp.length" (which evaluates to 6).
534 dp = efi_dp_concat(dp, &fp->dp, false);
535 free(buf);
536
537 return dp;
538 }
539
** CID 478333: Error handling issues (CHECKED_RETURN)
/lib/efi_loader/efi_firmware.c: 413 in efi_firmware_set_fmp_state_var()
________________________________________________________________________________________________________
*** 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
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2024-01-09 5:26 ` Sean Anderson
@ 2024-01-09 22:18 ` Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2024-01-09 22:18 UTC (permalink / raw)
To: Sean Anderson; +Cc: u-boot, Francis Laniel, Michael Trimarchi, Dario Binacchi
[-- Attachment #1: Type: text/plain, Size: 11301 bytes --]
On Tue, Jan 09, 2024 at 12:26:13AM -0500, Sean Anderson wrote:
> Comments on NAND stuff only.
>
> On 1/8/24 12:45, Tom Rini wrote:
> > ________________________________________________________________________________________________________
> > *** CID 477216: (BAD_SHIFT)
> > /drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
> > 3915
> > 3916 /*
> > 3917 * pages_per_block and blocks_per_lun may not be a
> > power-of-2 size
> > 3918 * (don't ask me who thought of this...). MTD assumes that these
> > 3919 * dimensions will be power-of-2, so just truncate the
> > remaining area.
> > 3920 */
> > > > > CID 477216: (BAD_SHIFT)
> > > > > In expression "1 << generic_fls((__u32)(__le32)p->pages_per_block) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->pages_per_block) - 1", is -1.
> > 3921 mtd->erasesize = 1 <<
> > (fls(le32_to_cpu(p->pages_per_block)) - 1);
> > 3922 mtd->erasesize *= mtd->writesize;
> > 3923
> > 3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> > 3925
> > 3926 /* See erasesize comment */
> > /drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
> > 3921 mtd->erasesize = 1 <<
> > (fls(le32_to_cpu(p->pages_per_block)) - 1);
> > 3922 mtd->erasesize *= mtd->writesize;
> > 3923
> > 3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> > 3925
> > 3926 /* See erasesize comment */
> > > > > CID 477216: (BAD_SHIFT)
> > > > > In expression "1 << generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", is -1.
> > 3927 chip->chipsize = 1 << (fls(le32_to_cpu(p->blocks_per_lun)) - 1);
> > 3928 chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> > 3929 chip->bits_per_cell = p->bits_per_cell;
> > 3930
> > 3931 if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
> > 3932 chip->options |= NAND_BUSWIDTH_16;
>
> Yeah, this looks like a bug.
>
> > ** CID 477215: Control flow issues (MISSING_BREAK)
> > /drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477215: Control flow issues (MISSING_BREAK)
> > /drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
> > 4972 pr_warn("No ECC functions supplied;
> > hardware ECC not possible\n");
> > 4973 BUG();
> > 4974 }
> > 4975 if (!ecc->read_page)
> > 4976 ecc->read_page = nand_read_page_hwecc_oob_first;
> > 4977
> > > > > CID 477215: Control flow issues (MISSING_BREAK)
> > > > > The case for value "NAND_ECC_HW" is not terminated by a "break" statement.
> > 4978 case NAND_ECC_HW:
> > 4979 /* Use standard hwecc read page function? */
> > 4980 if (!ecc->read_page)
> > 4981 ecc->read_page = nand_read_page_hwecc;
> > 4982 if (!ecc->write_page)
> > 4983 ecc->write_page = nand_write_page_hwecc;
>
> I think we just need a fallthrough comment here.
>
> > ** CID 477214: Integer handling issues (BAD_SHIFT)
> > /drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477214: Integer handling issues (BAD_SHIFT)
> > /drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
> > 4391
> > 4392 nand_decode_bbm_options(mtd, chip);
> > 4393
> > 4394 /* Calculate the address shift from the page size */
> > 4395 chip->page_shift = ffs(mtd->writesize) - 1;
> > 4396 /* Convert chipsize to number of pages per chip -1 */
> > > > > CID 477214: Integer handling issues (BAD_SHIFT)
> > > > > In expression "chip->chipsize >> chip->page_shift", shifting by a negative amount has undefined behavior. The shift amount, "chip->page_shift", is -1.
> > 4397 chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
> > 4398
> > 4399 chip->bbt_erase_shift = chip->phys_erase_shift =
> > 4400 ffs(mtd->erasesize) - 1;
> > 4401 if (chip->chipsize & 0xffffffff)
> > 4402 chip->chip_shift = ffs((unsigned)chip->chipsize) - 1;
>
> Buggy, but only when writesize is 0 (which is a bigger bug in the nand chip).
>
> > ** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
> > /test/dm/nand.c: 67 in dm_test_nand()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
> > /test/dm/nand.c: 67 in dm_test_nand()
> > 61 ops.ooblen = mtd->oobsize;
> > 62 ut_assertok(mtd_read_oob(mtd, mtd->erasesize, &ops));
> > 63 ut_asserteq(0, oob[mtd_to_nand(mtd)->badblockpos]);
> > 64
> > 65 /* Generate some data and write it */
> > 66 for (i = 0; i < size / sizeof(int); i++)
> > > > > CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
> > > > > "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
> > 67 gold[i] = rand();
> > 68 ut_assertok(nand_write_skip_bad(mtd, off, &length, NULL, U64_MAX,
> > 69 (void *)gold, 0));
> > 70 ut_asserteq(size, length);
> > 71
> > 72 /* Verify */
>
> Not a bug.
>
> > ** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
> > /drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
> > /drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
> > 1127
> > 1128 /* Prevent the bbt regions from erasing / writing */
> > 1129 mark_bbt_region(mtd, td);
> > 1130 if (md)
> > 1131 mark_bbt_region(mtd, md);
> > 1132
> > > > > CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
> > > > > Calling "vfree" frees "buf" using "vfree" but it should have been freed using "kfree". [Note: The source code implementation of the function has been overridden by a builtin model.]
> > 1133 vfree(buf);
> > 1134 return 0;
> > 1135
> > 1136 err:
> > 1137 kfree(this->bbt);
> > 1138 this->bbt = NULL;
>
> Not a bug, since these both call free().
>
> > ** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
> > /drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
> > /drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
> > 193 chip->tmp_dirty = true;
> > 194 for (i = 0; i < chip->err_steps; i++) {
> > 195 u32 bit_errors = chip->err_count;
> > 196 unsigned int j = chip->err_step_bits + chip->ecc_bits;
> > 197
> > 198 while (bit_errors) {
> > > > > CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
> > > > > "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
> > 199 unsigned int u = rand();
> > 200 float quot = 1ULL << 32;
> > 201
> > 202 do {
> > 203 quot *= j - bit_errors;
> > 204 quot /= j;
>
> Not a bug.
>
> > ** CID 477207: Control flow issues (MISSING_BREAK)
> > /drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477207: Control flow issues (MISSING_BREAK)
> > /drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
> > 4963 /*
> > 4964 * Check ECC mode, default to software if
> > 3byte/512byte hardware ECC is
> > 4965 * selected and we have 256 byte pagesize fallback to
> > software ECC
> > 4966 */
> > 4967
> > 4968 switch (ecc->mode) {
> > > > > CID 477207: Control flow issues (MISSING_BREAK)
> > > > > The case for value "NAND_ECC_HW_OOB_FIRST" is not terminated by a "break" statement.
> > 4969 case NAND_ECC_HW_OOB_FIRST:
> > 4970 /* Similar to NAND_ECC_HW, but a separate
> > read_page handle */
> > 4971 if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
> > 4972 pr_warn("No ECC functions supplied;
> > hardware ECC not possible\n");
> > 4973 BUG();
> > 4974 }
>
> need a fallthrough comment
>
> > ** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
> > /cmd/mtd.c: 88 in mtd_dump_device_buf()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
> > /cmd/mtd.c: 88 in mtd_dump_device_buf()
> > 82 printf("\nDump %d data bytes from 0x%08llx:\n",
> > 83 mtd->writesize, start_off + data_off);
> > 84 mtd_dump_buf(&buf[data_off],
> > 85 mtd->writesize, start_off + data_off);
> > 86
> > 87 if (woob) {
> > > > > CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
> > > > > Potentially overflowing expression "page * mtd->oobsize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
> > 88 u64 oob_off = page * mtd->oobsize;
> > 89
> > 90 printf("Dump %d OOB bytes from page at
> > 0x%08llx:\n",
> > 91 mtd->oobsize, start_off + data_off);
> > 92 mtd_dump_buf(&buf[len + oob_off],
> > 93 mtd->oobsize, 0);
>
> In the Linux MTD list [1], the largest this can be is 0xe0000000 for MT29F512G08CUCAB. That's worryingly
> close to overflow, so I'd say this is a bug.
Thanks, I've updated the not a bug ones in the dashboard.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2024-01-08 17:45 Tom Rini
@ 2024-01-09 5:26 ` Sean Anderson
2024-01-09 22:18 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Sean Anderson @ 2024-01-09 5:26 UTC (permalink / raw)
To: Tom Rini, u-boot, Francis Laniel; +Cc: Michael Trimarchi, Dario Binacchi
Comments on NAND stuff only.
On 1/8/24 12:45, Tom Rini wrote:
> ________________________________________________________________________________________________________
> *** CID 477216: (BAD_SHIFT)
> /drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
> 3915
> 3916 /*
> 3917 * pages_per_block and blocks_per_lun may not be a
> power-of-2 size
> 3918 * (don't ask me who thought of this...). MTD assumes that these
> 3919 * dimensions will be power-of-2, so just truncate the
> remaining area.
> 3920 */
>>>> CID 477216: (BAD_SHIFT)
>>>> In expression "1 << generic_fls((__u32)(__le32)p->pages_per_block) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->pages_per_block) - 1", is -1.
> 3921 mtd->erasesize = 1 <<
> (fls(le32_to_cpu(p->pages_per_block)) - 1);
> 3922 mtd->erasesize *= mtd->writesize;
> 3923
> 3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> 3925
> 3926 /* See erasesize comment */
> /drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
> 3921 mtd->erasesize = 1 <<
> (fls(le32_to_cpu(p->pages_per_block)) - 1);
> 3922 mtd->erasesize *= mtd->writesize;
> 3923
> 3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> 3925
> 3926 /* See erasesize comment */
>>>> CID 477216: (BAD_SHIFT)
>>>> In expression "1 << generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", is -1.
> 3927 chip->chipsize = 1 << (fls(le32_to_cpu(p->blocks_per_lun)) - 1);
> 3928 chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> 3929 chip->bits_per_cell = p->bits_per_cell;
> 3930
> 3931 if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
> 3932 chip->options |= NAND_BUSWIDTH_16;
Yeah, this looks like a bug.
> ** CID 477215: Control flow issues (MISSING_BREAK)
> /drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
>
>
> ________________________________________________________________________________________________________
> *** CID 477215: Control flow issues (MISSING_BREAK)
> /drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
> 4972 pr_warn("No ECC functions supplied;
> hardware ECC not possible\n");
> 4973 BUG();
> 4974 }
> 4975 if (!ecc->read_page)
> 4976 ecc->read_page = nand_read_page_hwecc_oob_first;
> 4977
>>>> CID 477215: Control flow issues (MISSING_BREAK)
>>>> The case for value "NAND_ECC_HW" is not terminated by a "break" statement.
> 4978 case NAND_ECC_HW:
> 4979 /* Use standard hwecc read page function? */
> 4980 if (!ecc->read_page)
> 4981 ecc->read_page = nand_read_page_hwecc;
> 4982 if (!ecc->write_page)
> 4983 ecc->write_page = nand_write_page_hwecc;
I think we just need a fallthrough comment here.
> ** CID 477214: Integer handling issues (BAD_SHIFT)
> /drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
>
>
> ________________________________________________________________________________________________________
> *** CID 477214: Integer handling issues (BAD_SHIFT)
> /drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
> 4391
> 4392 nand_decode_bbm_options(mtd, chip);
> 4393
> 4394 /* Calculate the address shift from the page size */
> 4395 chip->page_shift = ffs(mtd->writesize) - 1;
> 4396 /* Convert chipsize to number of pages per chip -1 */
>>>> CID 477214: Integer handling issues (BAD_SHIFT)
>>>> In expression "chip->chipsize >> chip->page_shift", shifting by a negative amount has undefined behavior. The shift amount, "chip->page_shift", is -1.
> 4397 chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
> 4398
> 4399 chip->bbt_erase_shift = chip->phys_erase_shift =
> 4400 ffs(mtd->erasesize) - 1;
> 4401 if (chip->chipsize & 0xffffffff)
> 4402 chip->chip_shift = ffs((unsigned)chip->chipsize) - 1;
Buggy, but only when writesize is 0 (which is a bigger bug in the nand chip).
> ** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
> /test/dm/nand.c: 67 in dm_test_nand()
>
>
> ________________________________________________________________________________________________________
> *** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
> /test/dm/nand.c: 67 in dm_test_nand()
> 61 ops.ooblen = mtd->oobsize;
> 62 ut_assertok(mtd_read_oob(mtd, mtd->erasesize, &ops));
> 63 ut_asserteq(0, oob[mtd_to_nand(mtd)->badblockpos]);
> 64
> 65 /* Generate some data and write it */
> 66 for (i = 0; i < size / sizeof(int); i++)
>>>> CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
> 67 gold[i] = rand();
> 68 ut_assertok(nand_write_skip_bad(mtd, off, &length, NULL, U64_MAX,
> 69 (void *)gold, 0));
> 70 ut_asserteq(size, length);
> 71
> 72 /* Verify */
Not a bug.
> ** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
> /drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
>
>
> ________________________________________________________________________________________________________
> *** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
> /drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
> 1127
> 1128 /* Prevent the bbt regions from erasing / writing */
> 1129 mark_bbt_region(mtd, td);
> 1130 if (md)
> 1131 mark_bbt_region(mtd, md);
> 1132
>>>> CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
>>>> Calling "vfree" frees "buf" using "vfree" but it should have been freed using "kfree". [Note: The source code implementation of the function has been overridden by a builtin model.]
> 1133 vfree(buf);
> 1134 return 0;
> 1135
> 1136 err:
> 1137 kfree(this->bbt);
> 1138 this->bbt = NULL;
Not a bug, since these both call free().
> ** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
> /drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
>
>
> ________________________________________________________________________________________________________
> *** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
> /drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
> 193 chip->tmp_dirty = true;
> 194 for (i = 0; i < chip->err_steps; i++) {
> 195 u32 bit_errors = chip->err_count;
> 196 unsigned int j = chip->err_step_bits + chip->ecc_bits;
> 197
> 198 while (bit_errors) {
>>>> CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
> 199 unsigned int u = rand();
> 200 float quot = 1ULL << 32;
> 201
> 202 do {
> 203 quot *= j - bit_errors;
> 204 quot /= j;
Not a bug.
> ** CID 477207: Control flow issues (MISSING_BREAK)
> /drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
>
>
> ________________________________________________________________________________________________________
> *** CID 477207: Control flow issues (MISSING_BREAK)
> /drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
> 4963 /*
> 4964 * Check ECC mode, default to software if
> 3byte/512byte hardware ECC is
> 4965 * selected and we have 256 byte pagesize fallback to
> software ECC
> 4966 */
> 4967
> 4968 switch (ecc->mode) {
>>>> CID 477207: Control flow issues (MISSING_BREAK)
>>>> The case for value "NAND_ECC_HW_OOB_FIRST" is not terminated by a "break" statement.
> 4969 case NAND_ECC_HW_OOB_FIRST:
> 4970 /* Similar to NAND_ECC_HW, but a separate
> read_page handle */
> 4971 if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
> 4972 pr_warn("No ECC functions supplied;
> hardware ECC not possible\n");
> 4973 BUG();
> 4974 }
need a fallthrough comment
> ** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
> /cmd/mtd.c: 88 in mtd_dump_device_buf()
>
>
> ________________________________________________________________________________________________________
> *** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
> /cmd/mtd.c: 88 in mtd_dump_device_buf()
> 82 printf("\nDump %d data bytes from 0x%08llx:\n",
> 83 mtd->writesize, start_off + data_off);
> 84 mtd_dump_buf(&buf[data_off],
> 85 mtd->writesize, start_off + data_off);
> 86
> 87 if (woob) {
>>>> CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>>> Potentially overflowing expression "page * mtd->oobsize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
> 88 u64 oob_off = page * mtd->oobsize;
> 89
> 90 printf("Dump %d OOB bytes from page at
> 0x%08llx:\n",
> 91 mtd->oobsize, start_off + data_off);
> 92 mtd_dump_buf(&buf[len + oob_off],
> 93 mtd->oobsize, 0);
In the Linux MTD list [1], the largest this can be is 0xe0000000 for MT29F512G08CUCAB. That's worryingly
close to overflow, so I'd say this is a bug.
--Sean
[1] http://linux-mtd.infradead.org/nand-data/nanddata.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-01-08 17:45 Tom Rini
2024-01-09 5:26 ` Sean Anderson
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2024-01-08 17:45 UTC (permalink / raw)
To: u-boot, Francis Laniel, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 25689 bytes --]
Hey all,
Now that I've merged next I've re-run Coverity to get a start on issues
that've been added since last run. The report isn't complete because of
the number of issues, sadly, but if someone is interested in a specific
area contact me off-list and I can provide access to the dashboard.
For the hush related issues, this would be a good chance to work with
upstream and then backport the changes I suspect.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Jan 8, 2024 at 12:24 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To: <tom.rini@gmail.com>
Hi,
Please find the latest report on new defect(s) introduced to Das
U-Boot found with Coverity Scan.
41 new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 20 of 41 defect(s)
** CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5518 in parse_dollar()
________________________________________________________________________________________________________
*** CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5518 in parse_dollar()
5512 break;
5513 if (--cnt == 0)
5514 goto bad_dollar_syntax;
5515 if (len_single_ch != '#' &&
strchr(VAR_SUBST_OPS, ch))
5516 /* ${NN<op>...} is valid */
5517 goto eat_until_closing;
>>> CID 477217: Memory - illegal accesses (NEGATIVE_RETURNS)
>>> Using variable "ch" as an index to array "_ctype".
5518 if (!isdigit(ch))
5519 goto bad_dollar_syntax;
5520 }
5521 } else
5522 while (1) {
5523 unsigned pos;
** CID 477216: (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
/drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
________________________________________________________________________________________________________
*** CID 477216: (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 3921 in nand_flash_detect_onfi()
3915
3916 /*
3917 * pages_per_block and blocks_per_lun may not be a
power-of-2 size
3918 * (don't ask me who thought of this...). MTD assumes that these
3919 * dimensions will be power-of-2, so just truncate the
remaining area.
3920 */
>>> CID 477216: (BAD_SHIFT)
>>> In expression "1 << generic_fls((__u32)(__le32)p->pages_per_block) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->pages_per_block) - 1", is -1.
3921 mtd->erasesize = 1 <<
(fls(le32_to_cpu(p->pages_per_block)) - 1);
3922 mtd->erasesize *= mtd->writesize;
3923
3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
3925
3926 /* See erasesize comment */
/drivers/mtd/nand/raw/nand_base.c: 3927 in nand_flash_detect_onfi()
3921 mtd->erasesize = 1 <<
(fls(le32_to_cpu(p->pages_per_block)) - 1);
3922 mtd->erasesize *= mtd->writesize;
3923
3924 mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
3925
3926 /* See erasesize comment */
>>> CID 477216: (BAD_SHIFT)
>>> In expression "1 << generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", shifting by a negative amount has undefined behavior. The shift amount, "generic_fls((__u32)(__le32)p->blocks_per_lun) - 1", is -1.
3927 chip->chipsize = 1 << (fls(le32_to_cpu(p->blocks_per_lun)) - 1);
3928 chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
3929 chip->bits_per_cell = p->bits_per_cell;
3930
3931 if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
3932 chip->options |= NAND_BUSWIDTH_16;
** CID 477215: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
________________________________________________________________________________________________________
*** CID 477215: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4978 in nand_scan_tail()
4972 pr_warn("No ECC functions supplied;
hardware ECC not possible\n");
4973 BUG();
4974 }
4975 if (!ecc->read_page)
4976 ecc->read_page = nand_read_page_hwecc_oob_first;
4977
>>> CID 477215: Control flow issues (MISSING_BREAK)
>>> The case for value "NAND_ECC_HW" is not terminated by a "break" statement.
4978 case NAND_ECC_HW:
4979 /* Use standard hwecc read page function? */
4980 if (!ecc->read_page)
4981 ecc->read_page = nand_read_page_hwecc;
4982 if (!ecc->write_page)
4983 ecc->write_page = nand_write_page_hwecc;
** CID 477214: Integer handling issues (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
________________________________________________________________________________________________________
*** CID 477214: Integer handling issues (BAD_SHIFT)
/drivers/mtd/nand/raw/nand_base.c: 4397 in nand_detect()
4391
4392 nand_decode_bbm_options(mtd, chip);
4393
4394 /* Calculate the address shift from the page size */
4395 chip->page_shift = ffs(mtd->writesize) - 1;
4396 /* Convert chipsize to number of pages per chip -1 */
>>> CID 477214: Integer handling issues (BAD_SHIFT)
>>> In expression "chip->chipsize >> chip->page_shift", shifting by a negative amount has undefined behavior. The shift amount, "chip->page_shift", is -1.
4397 chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
4398
4399 chip->bbt_erase_shift = chip->phys_erase_shift =
4400 ffs(mtd->erasesize) - 1;
4401 if (chip->chipsize & 0xffffffff)
4402 chip->chip_shift = ffs((unsigned)chip->chipsize) - 1;
** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in dm_test_nand()
________________________________________________________________________________________________________
*** CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in dm_test_nand()
61 ops.ooblen = mtd->oobsize;
62 ut_assertok(mtd_read_oob(mtd, mtd->erasesize, &ops));
63 ut_asserteq(0, oob[mtd_to_nand(mtd)->badblockpos]);
64
65 /* Generate some data and write it */
66 for (i = 0; i < size / sizeof(int); i++)
>>> CID 477213: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
67 gold[i] = rand();
68 ut_assertok(nand_write_skip_bad(mtd, off, &length, NULL, U64_MAX,
69 (void *)gold, 0));
70 ut_asserteq(size, length);
71
72 /* Verify */
** CID 477212: Incorrect expression (SIZEOF_MISMATCH)
/lib/smbios.c: 595 in write_smbios_table()
________________________________________________________________________________________________________
*** CID 477212: Incorrect expression (SIZEOF_MISMATCH)
/lib/smbios.c: 595 in write_smbios_table()
589 * sandbox's DRAM buffer.
590 */
591 table_addr = (ulong)map_sysmem(tables, 0);
592
593 /* now go back and write the SMBIOS3 header */
594 se = map_sysmem(start_addr, sizeof(struct smbios_entry));
>>> CID 477212: Incorrect expression (SIZEOF_MISMATCH)
>>> Passing argument "se" of type "struct smbios3_entry *" and argument "31UL" ("sizeof (struct smbios_entry)") to function "memset" is suspicious because a multiple of "sizeof (struct smbios3_entry) /*24*/" is expected.
595 memset(se, '\0', sizeof(struct smbios_entry));
596 memcpy(se->anchor, "_SM3_", 5);
597 se->length = sizeof(struct smbios3_entry);
598 se->major_ver = SMBIOS_MAJOR_VER;
599 se->minor_ver = SMBIOS_MINOR_VER;
600 se->doc_rev = 0;
** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
/drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
________________________________________________________________________________________________________
*** CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
/drivers/mtd/nand/raw/nand_bbt.c: 1133 in nand_scan_bbt()
1127
1128 /* Prevent the bbt regions from erasing / writing */
1129 mark_bbt_region(mtd, td);
1130 if (md)
1131 mark_bbt_region(mtd, md);
1132
>>> CID 477211: API usage errors (ALLOC_FREE_MISMATCH)
>>> Calling "vfree" frees "buf" using "vfree" but it should have been freed using "kfree". [Note: The source code implementation of the function has been overridden by a builtin model.]
1133 vfree(buf);
1134 return 0;
1135
1136 err:
1137 kfree(this->bbt);
1138 this->bbt = NULL;
** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
/drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
________________________________________________________________________________________________________
*** CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
/drivers/mtd/nand/raw/sand_nand.c: 199 in sand_nand_read()
193 chip->tmp_dirty = true;
194 for (i = 0; i < chip->err_steps; i++) {
195 u32 bit_errors = chip->err_count;
196 unsigned int j = chip->err_step_bits + chip->ecc_bits;
197
198 while (bit_errors) {
>>> CID 477210: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
199 unsigned int u = rand();
200 float quot = 1ULL << 32;
201
202 do {
203 quot *= j - bit_errors;
204 quot /= j;
** CID 477209: Memory - illegal accesses (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477209: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 4434 in reserved_word()
4428 str = old->as_string.data + len;
4429 if (str > old->as_string.data)
4430 str--; /* skip whitespace
after keyword */
4431 while (str > old->as_string.data &&
isalpha(str[-1]))
4432 str--;
4433 /* Ugh, we're done with this horrid hack */
>>> CID 477209: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "str" to "sandbox_strdup", which expects a null-terminated string.
4434 old->command->group_as_string = xstrdup(str);
4435 debug_printf_parse("pop, remembering as:'%s'\n",
4436 old->command->group_as_string);
4437 }
4438 # endif
4439 *ctx = *old; /* physical copy */
** CID 477208: Memory - illegal accesses (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477208: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 7660 in expand_variables()
7654 output.o_expflags = expflags;
7655
7656 n = 0;
7657 for (;;) {
7658 /* go to next list[n] */
7659 output.ended_in_ifs = 0;
>>> CID 477208: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "output.data" to "o_save_ptr", which expects a null-terminated string.
7660 n = o_save_ptr(&output, n);
7661
7662 if (!*argv)
7663 break;
7664
7665 /* expand argv[i] */
** CID 477207: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
________________________________________________________________________________________________________
*** CID 477207: Control flow issues (MISSING_BREAK)
/drivers/mtd/nand/raw/nand_base.c: 4969 in nand_scan_tail()
4963 /*
4964 * Check ECC mode, default to software if
3byte/512byte hardware ECC is
4965 * selected and we have 256 byte pagesize fallback to
software ECC
4966 */
4967
4968 switch (ecc->mode) {
>>> CID 477207: Control flow issues (MISSING_BREAK)
>>> The case for value "NAND_ECC_HW_OOB_FIRST" is not terminated by a "break" statement.
4969 case NAND_ECC_HW_OOB_FIRST:
4970 /* Similar to NAND_ECC_HW, but a separate
read_page handle */
4971 if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
4972 pr_warn("No ECC functions supplied;
hardware ECC not possible\n");
4973 BUG();
4974 }
** CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5544 in parse_dollar()
________________________________________________________________________________________________________
*** CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
/common/cli_hush_upstream.c: 5544 in parse_dollar()
5538 * So, we need to authorize # to appear inside
5539 * variable name and then expand this variable.
5540 * NOTE Having # in variable name is
not permitted in
5541 * upstream hush but expansion will be
done (even though
5542 * the result will be empty).
5543 */
>>> CID 477206: Memory - illegal accesses (NEGATIVE_RETURNS)
>>> Using variable "ch" as an index to array "_ctype".
5544 if (!isalnum(ch) && ch != '_' && ch != '#') {
5545 #endif /* __U_BOOT__ */
5546 unsigned end_ch;
5547 #ifndef __U_BOOT__
5548 unsigned char last_ch;
5549 #endif /* !__U_BOOT__ */
** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 88 in mtd_dump_device_buf()
________________________________________________________________________________________________________
*** CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 88 in mtd_dump_device_buf()
82 printf("\nDump %d data bytes from 0x%08llx:\n",
83 mtd->writesize, start_off + data_off);
84 mtd_dump_buf(&buf[data_off],
85 mtd->writesize, start_off + data_off);
86
87 if (woob) {
>>> CID 477205: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "page * mtd->oobsize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
88 u64 oob_off = page * mtd->oobsize;
89
90 printf("Dump %d OOB bytes from page at
0x%08llx:\n",
91 mtd->oobsize, start_off + data_off);
92 mtd_dump_buf(&buf[len + oob_off],
93 mtd->oobsize, 0);
** CID 477204: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 10553 in run_list()
________________________________________________________________________________________________________
*** CID 477204: Memory - illegal accesses (STRING_NULL)
/common/cli_hush_upstream.c: 10553 in run_list()
10547 /* We cannot use xasprintf, so we emulate it. */
10548 char *full_var;
10549 char *var = pi->cmds[0].argv[0];
10550 char *val = *for_lcur++;
10551
10552 /* + 1 to take into account =. */
>>> CID 477204: Memory - illegal accesses (STRING_NULL)
>>> Passing unterminated string "val" to "strlen", which expects a null-terminated string. [Note: The source code implementation of the function has been overridden by a builtin model.]
10553 full_var = xmalloc(strlen(var) +
strlen(val) + 1);
10554 sprintf(full_var, "%s=%s", var, val);
10555
10556 set_local_var_modern(full_var, /*flag:*/ 0);
10557 #endif /* __U_BOOT__ */
10558 continue;
** CID 477203: (UNINIT)
/boot/bootm.c: 705 in bootm_load_os()
/boot/bootm.c: 713 in bootm_load_os()
________________________________________________________________________________________________________
*** CID 477203: (UNINIT)
/boot/bootm.c: 705 in bootm_load_os()
699 printf("Failed to prep arm64 kernel
(err=%d)\n", ret);
700 return BOOTM_ERR_RESET;
701 }
702
703 /* Handle BOOTM_STATE_LOADOS */
704 if (relocated_addr != load) {
>>> CID 477203: (UNINIT)
>>> Using uninitialized value "image_size".
705 printf("Moving Image from 0x%lx to
0x%lx, end=%lx\n",
706 load, relocated_addr,
707 relocated_addr + image_size);
708 memmove((void *)relocated_addr,
load_buf, image_size);
709 }
710
/boot/bootm.c: 713 in bootm_load_os()
707 relocated_addr + image_size);
708 memmove((void *)relocated_addr,
load_buf, image_size);
709 }
710
711 images->ep = relocated_addr;
712 images->os.start = relocated_addr;
>>> CID 477203: (UNINIT)
>>> Using uninitialized value "image_size".
713 images->os.end = relocated_addr + image_size;
714 }
715
716 lmb_reserve(&images->lmb, images->os.load, (load_end -
717 images->os.load));
718 return 0;
** CID 477202: Null pointer dereferences (FORWARD_NULL)
________________________________________________________________________________________________________
*** CID 477202: Null pointer dereferences (FORWARD_NULL)
/common/cli_hush_upstream.c: 4425 in reserved_word()
4419 * with "if " remaining in old->as_string!
4420 */
4421 {
4422 char *str;
4423 int len = old->as_string.length;
4424 /* Concatenate halves */
>>> CID 477202: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "ctx->as_string.data" to "o_addstr", which dereferences it.
4425 o_addstr(&old->as_string, ctx->as_string.data);
4426 o_free(&ctx->as_string);
4427 /* Find where leading keyword starts
in first half */
4428 str = old->as_string.data + len;
4429 if (str > old->as_string.data)
4430 str--; /* skip whitespace
after keyword */
** CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 80 in mtd_dump_device_buf()
________________________________________________________________________________________________________
*** CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/cmd/mtd.c: 80 in mtd_dump_device_buf()
74 mtd->type == MTD_MLCNANDFLASH;
75 int npages = mtd_len_to_pages(mtd, len);
76 uint page;
77
78 if (has_pages) {
79 for (page = 0; page < npages; page++) {
>>> CID 477201: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "page * mtd->writesize" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
80 u64 data_off = page * mtd->writesize;
81
82 printf("\nDump %d data bytes from 0x%08llx:\n",
83 mtd->writesize, start_off + data_off);
84 mtd_dump_buf(&buf[data_off],
85 mtd->writesize, start_off + data_off);
** CID 477200: Security best practices violations (STRING_OVERFLOW)
/boot/bootm.c: 499 in bootm_find_images()
________________________________________________________________________________________________________
*** CID 477200: Security best practices violations (STRING_OVERFLOW)
/boot/bootm.c: 499 in bootm_find_images()
493 int ret;
494
495 if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
496 /* Look for an Android boot image */
497 buf = map_sysmem(images.os.start, 0);
498 if (buf && genimg_get_format(buf) ==
IMAGE_FORMAT_ANDROID) {
>>> CID 477200: Security best practices violations (STRING_OVERFLOW)
>>> You might overrun the 17-character fixed-size string "addr_str" by copying the return value of "simple_xtoa" without checking the length.
499 strcpy(addr_str, simple_xtoa(img_addr));
500 select = addr_str;
501 }
502 }
503
504 if (conf_ramdisk)
** CID 477199: (STRING_NULL)
________________________________________________________________________________________________________
*** CID 477199: (STRING_NULL)
/common/cli_hush_upstream.c: 10315 in run_pipe()
10309 if (cmd_no < pi->num_cmds)
10310 close(pipefds.wr);
10311 /* Pass read (output) pipe end to next iteration */
10312 next_infd = pipefds.rd;
10313 #else /* __U_BOOT__ */
10314 /* Process the command */
>>> CID 477199: (STRING_NULL)
>>> Passing unterminated string "*command->argv" to "cmd_process", which expects a null-terminated string.
10315 rcode = cmd_process(G.do_repeat ? CMD_FLAG_REPEAT : 0,
10316 command->argc, command->argv,
10317 &(G.flag_repeat), NULL);
10318
10319 if (argv_expanded) {
10320 /*
/common/cli_hush_upstream.c: 9984 in run_pipe()
9978 }
9979 #endif
9980 debug_printf_env("set shell
var:'%s'->'%s'\n", *argv, p);
9981 #ifndef __U_BOOT__
9982 if (set_local_var0(p)) {
9983 #else /* __U_BOOT__ */
>>> CID 477199: (STRING_NULL)
>>> Passing unterminated string "p" to "set_local_var_modern", which expects a null-terminated string.
9984 if (set_local_var_modern(p,
/*flag:*/ 0)) {
9985 #endif
9986 /* assignment to
readonly var / putenv error? */
9987 rcode = 1;
9988 }
9989 i++;
** CID 477198: Control flow issues (DEADCODE)
/cmd/bootflow.c: 547 in do_bootflow_cmdline()
________________________________________________________________________________________________________
*** CID 477198: Control flow issues (DEADCODE)
/cmd/bootflow.c: 547 in do_bootflow_cmdline()
541 }
542
543 op = argv[1];
544 arg = argv[2];
545 if (*op == 's') {
546 if (argc < 3)
>>> CID 477198: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return CMD_RET_USAGE;".
547 return CMD_RET_USAGE;
548 val = argv[3] ?: (const char *)BOOTFLOWCL_EMPTY;
549 }
550
551 switch (*op) {
552 case 'c': /* clear */
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-10-25 15:15 ` Tom Rini
@ 2023-10-31 14:21 ` Abdellatif El Khlifi
0 siblings, 0 replies; 36+ messages in thread
From: Abdellatif El Khlifi @ 2023-10-31 14:21 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, nd
Hi Tom,
> > > > > ________________________________________________________________________________________________________
> > > > > *** CID 464361: Control flow issues (DEADCODE)
> > > > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> > > > > 142
> > > > > 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
> > > > > 144 return -EINVAL;
> > > > > 145
> > > > > 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
> > > > > 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> > > > > >>> CID 464361: Control flow issues (DEADCODE)
> > > > > >>> Execution cannot reach this statement: "return -22;".
> > > > > 148 return -EINVAL;
> > > >
> > > > This is a false positive.
> > > >
> > > > abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
> > > >
> > > > This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
> > > >
> > > > ffa_print_error_log(0x50, ...); /* exceeding lower bound */
> > > > ffa_print_error_log(0x80, ...); /* exceeding upper bound */
> > > >
> > > > In these cases "return -EINVAL;" is executed.
> > >
> > > So those invalid values aren't caught by the previous check that ffa_id
> > > falls within FFA_FIRST_ID to FFA_LAST_ID ?
> >
> > I had a closer look at that and I agree that the deadcode defect is legitimate.
> > I already provided a fix [1].
> >
> > [1]: https://lore.kernel.org/all/20231020131533.239591-1-abdellatif.elkhlifi@arm.com/
>
> Ah thanks. I had seen that posted but not put that together with this
> email and assumed it was addressing something you hadn't talked about
> here because you agreed with it being an issue. I will pick up the
> above patch soon then.
Thank you very much.
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-10-25 15:12 ` Abdellatif El Khlifi
@ 2023-10-25 15:15 ` Tom Rini
2023-10-31 14:21 ` Abdellatif El Khlifi
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2023-10-25 15:15 UTC (permalink / raw)
To: Abdellatif El Khlifi; +Cc: nd, u-boot, xueliang.zhong
[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]
On Wed, Oct 25, 2023 at 04:12:37PM +0100, Abdellatif El Khlifi wrote:
> Hi Tom,
>
> > > > ________________________________________________________________________________________________________
> > > > *** CID 464361: Control flow issues (DEADCODE)
> > > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> > > > 142
> > > > 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
> > > > 144 return -EINVAL;
> > > > 145
> > > > 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
> > > > 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> > > > >>> CID 464361: Control flow issues (DEADCODE)
> > > > >>> Execution cannot reach this statement: "return -22;".
> > > > 148 return -EINVAL;
> > >
> > > This is a false positive.
> > >
> > > abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
> > >
> > > This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
> > >
> > > ffa_print_error_log(0x50, ...); /* exceeding lower bound */
> > > ffa_print_error_log(0x80, ...); /* exceeding upper bound */
> > >
> > > In these cases "return -EINVAL;" is executed.
> >
> > So those invalid values aren't caught by the previous check that ffa_id
> > falls within FFA_FIRST_ID to FFA_LAST_ID ?
>
> I had a closer look at that and I agree that the deadcode defect is legitimate.
> I already provided a fix [1].
>
> [1]: https://lore.kernel.org/all/20231020131533.239591-1-abdellatif.elkhlifi@arm.com/
Ah thanks. I had seen that posted but not put that together with this
email and assumed it was addressing something you hadn't talked about
here because you agreed with it being an issue. I will pick up the
above patch soon then.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-10-25 14:57 ` Tom Rini
@ 2023-10-25 15:12 ` Abdellatif El Khlifi
2023-10-25 15:15 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Abdellatif El Khlifi @ 2023-10-25 15:12 UTC (permalink / raw)
To: Tom Rini; +Cc: nd, trini, u-boot, xueliang.zhong
Hi Tom,
> > > ________________________________________________________________________________________________________
> > > *** CID 464361: Control flow issues (DEADCODE)
> > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> > > 142
> > > 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
> > > 144 return -EINVAL;
> > > 145
> > > 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
> > > 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> > > >>> CID 464361: Control flow issues (DEADCODE)
> > > >>> Execution cannot reach this statement: "return -22;".
> > > 148 return -EINVAL;
> >
> > This is a false positive.
> >
> > abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
> >
> > This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
> >
> > ffa_print_error_log(0x50, ...); /* exceeding lower bound */
> > ffa_print_error_log(0x80, ...); /* exceeding upper bound */
> >
> > In these cases "return -EINVAL;" is executed.
>
> So those invalid values aren't caught by the previous check that ffa_id
> falls within FFA_FIRST_ID to FFA_LAST_ID ?
I had a closer look at that and I agree that the deadcode defect is legitimate.
I already provided a fix [1].
[1]: https://lore.kernel.org/all/20231020131533.239591-1-abdellatif.elkhlifi@arm.com/
>
> > > ...
> > > ________________________________________________________________________________________________________
> > > *** CID 464360: Control flow issues (NO_EFFECT)
> > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
> > > 201 major = GET_FFA_MAJOR_VERSION(res.a0);
> > > 202 minor = GET_FFA_MINOR_VERSION(res.a0);
> > > 203
> > > 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n",
> > > 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor);
> > > 206
> > > >>> CID 464360: Control flow issues (NO_EFFECT)
> > > >>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".
> > > 207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
> >
> > Providing the facts that:
> >
> > #define FFA_MINOR_VERSION (0)
> > u16 minor;
> >
> > Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
> >
> > However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" ,
> > non compatible versions could pass which we don't want.
> >
> > To keep this code scalable, I think it's better to keep this condition.
>
> OK, thanks this makes sense as an intentional change for future sanity
> checking.
>
> > > ________________________________________________________________________________________________________
> > > *** CID 464359: (PASS_BY_VALUE)
> > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
> > > 162 * @args: FF-A ABI arguments to be copied to Xn registers
> > > 163 * @res: FF-A ABI return data to be copied from Xn registers
> > > 164 *
> > > 165 * Calls low level SMC implementation.
> > > 166 * This function should be implemented by the user driver.
> > > 167 */
> > > >>> CID 464359: (PASS_BY_VALUE)
> > > >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> > > 168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
> >
> > We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A
> > driver to make porting code easier.
> >
> > In Linux, args is passed by value [1].
> > ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
> >
> > [1]: invoke_ffa_fn arguments in the Linux FF-A driver
> >
> > https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L115
> > https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L54
> > https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/common.h#L15
> >
> > [2]: include/linux/arm-smccc.h
>
> So this is intentional, OK.
>
> >
> > > 169 {
> > > 170 }
> > > 171
> > > 172 /**
> > > 173 * ffa_get_version_hdlr() - FFA_VERSION handler function
> > > /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
> > > 667 * invoke_ffa_fn() - SMC wrapper
> > > 668 * @args: FF-A ABI arguments to be copied to Xn registers
> > > 669 * @res: FF-A ABI return data to be copied from Xn registers
> > > 670 *
> > > 671 * Calls the emulated SMC call.
> > > 672 */
> > > >>> CID 464359: (PASS_BY_VALUE)
> > > >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> > > 673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
> >
> > Same feedback as above.
>
> Thanks. I'll update the last 3 CIDs shortly.
Thanks Tom :)
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-10-20 11:57 ` Abdellatif El Khlifi
@ 2023-10-25 14:57 ` Tom Rini
2023-10-25 15:12 ` Abdellatif El Khlifi
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2023-10-25 14:57 UTC (permalink / raw)
To: Abdellatif El Khlifi; +Cc: u-boot, nd, xueliang.zhong
[-- Attachment #1: Type: text/plain, Size: 4911 bytes --]
On Fri, Oct 20, 2023 at 12:57:47PM +0100, Abdellatif El Khlifi wrote:
> Hi Tom,
>
> > ________________________________________________________________________________________________________
> > *** CID 464361: Control flow issues (DEADCODE)
> > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> > 142
> > 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
> > 144 return -EINVAL;
> > 145
> > 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
> > 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> > >>> CID 464361: Control flow issues (DEADCODE)
> > >>> Execution cannot reach this statement: "return -22;".
> > 148 return -EINVAL;
>
> This is a false positive.
>
> abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
>
> This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
>
> ffa_print_error_log(0x50, ...); /* exceeding lower bound */
> ffa_print_error_log(0x80, ...); /* exceeding upper bound */
>
> In these cases "return -EINVAL;" is executed.
So those invalid values aren't caught by the previous check that ffa_id
falls within FFA_FIRST_ID to FFA_LAST_ID ?
> > ...
> > ________________________________________________________________________________________________________
> > *** CID 464360: Control flow issues (NO_EFFECT)
> > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
> > 201 major = GET_FFA_MAJOR_VERSION(res.a0);
> > 202 minor = GET_FFA_MINOR_VERSION(res.a0);
> > 203
> > 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n",
> > 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor);
> > 206
> > >>> CID 464360: Control flow issues (NO_EFFECT)
> > >>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".
> > 207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
>
> Providing the facts that:
>
> #define FFA_MINOR_VERSION (0)
> u16 minor;
>
> Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
>
> However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" ,
> non compatible versions could pass which we don't want.
>
> To keep this code scalable, I think it's better to keep this condition.
OK, thanks this makes sense as an intentional change for future sanity
checking.
> > ________________________________________________________________________________________________________
> > *** CID 464359: (PASS_BY_VALUE)
> > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
> > 162 * @args: FF-A ABI arguments to be copied to Xn registers
> > 163 * @res: FF-A ABI return data to be copied from Xn registers
> > 164 *
> > 165 * Calls low level SMC implementation.
> > 166 * This function should be implemented by the user driver.
> > 167 */
> > >>> CID 464359: (PASS_BY_VALUE)
> > >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> > 168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
>
> We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A
> driver to make porting code easier.
>
> In Linux, args is passed by value [1].
> ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
>
> [1]: invoke_ffa_fn arguments in the Linux FF-A driver
>
> https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L115
> https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L54
> https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/common.h#L15
>
> [2]: include/linux/arm-smccc.h
So this is intentional, OK.
>
> > 169 {
> > 170 }
> > 171
> > 172 /**
> > 173 * ffa_get_version_hdlr() - FFA_VERSION handler function
> > /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
> > 667 * invoke_ffa_fn() - SMC wrapper
> > 668 * @args: FF-A ABI arguments to be copied to Xn registers
> > 669 * @res: FF-A ABI return data to be copied from Xn registers
> > 670 *
> > 671 * Calls the emulated SMC call.
> > 672 */
> > >>> CID 464359: (PASS_BY_VALUE)
> > >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> > 673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
>
> Same feedback as above.
Thanks. I'll update the last 3 CIDs shortly.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-08-21 21:09 Tom Rini
2023-08-24 9:27 ` Abdellatif El Khlifi
@ 2023-10-20 11:57 ` Abdellatif El Khlifi
2023-10-25 14:57 ` Tom Rini
1 sibling, 1 reply; 36+ messages in thread
From: Abdellatif El Khlifi @ 2023-10-20 11:57 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, nd, xueliang.zhong
Hi Tom,
> ________________________________________________________________________________________________________
> *** CID 464361: Control flow issues (DEADCODE)
> /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> 142
> 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
> 144 return -EINVAL;
> 145
> 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
> 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> >>> CID 464361: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "return -22;".
> 148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */
ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
> ...
> ________________________________________________________________________________________________________
> *** CID 464360: Control flow issues (NO_EFFECT)
> /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
> 201 major = GET_FFA_MAJOR_VERSION(res.a0);
> 202 minor = GET_FFA_MINOR_VERSION(res.a0);
> 203
> 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n",
> 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor);
> 206
> >>> CID 464360: Control flow issues (NO_EFFECT)
> >>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".
> 207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
Providing the facts that:
#define FFA_MINOR_VERSION (0)
u16 minor;
Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" ,
non compatible versions could pass which we don't want.
To keep this code scalable, I think it's better to keep this condition.
> ...
> ________________________________________________________________________________________________________
> *** CID 464359: (PASS_BY_VALUE)
> /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
> 162 * @args: FF-A ABI arguments to be copied to Xn registers
> 163 * @res: FF-A ABI return data to be copied from Xn registers
> 164 *
> 165 * Calls low level SMC implementation.
> 166 * This function should be implemented by the user driver.
> 167 */
> >>> CID 464359: (PASS_BY_VALUE)
> >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> 168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A
driver to make porting code easier.
In Linux, args is passed by value [1].
ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
[1]: invoke_ffa_fn arguments in the Linux FF-A driver
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L115
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/driver.c#L54
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/common.h#L15
[2]: include/linux/arm-smccc.h
> 169 {
> 170 }
> 171
> 172 /**
> 173 * ffa_get_version_hdlr() - FFA_VERSION handler function
> /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
> 667 * invoke_ffa_fn() - SMC wrapper
> 668 * @args: FF-A ABI arguments to be copied to Xn registers
> 669 * @res: FF-A ABI return data to be copied from Xn registers
> 670 *
> 671 * Calls the emulated SMC call.
> 672 */
> >>> CID 464359: (PASS_BY_VALUE)
> >>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
> 673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
Same feedback as above.
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-08-28 16:09 ` Alvaro Fernando García
@ 2023-08-28 16:11 ` Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2023-08-28 16:11 UTC (permalink / raw)
To: Alvaro Fernando García; +Cc: Abdellatif El Khlifi, U-Boot Mailing List, nd
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
On Mon, Aug 28, 2023 at 01:09:17PM -0300, Alvaro Fernando García wrote:
> Hello,
>
> El jue, 24 ago. 2023 06:27, Abdellatif El Khlifi <
> abdellatif.elkhlifi@arm.com> escribió:
>
> > Hi Tom,
> >
> > > Here's the latest report
> > >
> > > ---------- Forwarded message ---------
> > > From: <scan-admin@coverity.com>
> > > Date: Mon, Aug 21, 2023 at 4:30 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.
> > >
> > > 4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> > >
> > >
> > > New defect(s) Reported-by: Coverity Scan
> > > Showing 4 of 4 defect(s)
> > >
> > >
> > > ** CID 464361: Control flow issues (DEADCODE)
> > > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
> >
> > Well received, I started working on that.
> > I'll provide a fix after coming back fom holidays (mid September)
> >
> > Cheers,
> > Abdellatif
> >
>
> Is there something I could do to help with this?
Everyone is free to work on these issues, yes.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-08-24 9:27 ` Abdellatif El Khlifi
@ 2023-08-28 16:09 ` Alvaro Fernando García
2023-08-28 16:11 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Alvaro Fernando García @ 2023-08-28 16:09 UTC (permalink / raw)
To: Abdellatif El Khlifi; +Cc: Tom Rini, U-Boot Mailing List, nd
Hello,
El jue, 24 ago. 2023 06:27, Abdellatif El Khlifi <
abdellatif.elkhlifi@arm.com> escribió:
> Hi Tom,
>
> > Here's the latest report
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Aug 21, 2023 at 4:30 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.
> >
> > 4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 4 of 4 defect(s)
> >
> >
> > ** CID 464361: Control flow issues (DEADCODE)
> > /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
>
> Well received, I started working on that.
> I'll provide a fix after coming back fom holidays (mid September)
>
> Cheers,
> Abdellatif
>
Is there something I could do to help with this?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-08-21 21:09 Tom Rini
@ 2023-08-24 9:27 ` Abdellatif El Khlifi
2023-08-28 16:09 ` Alvaro Fernando García
2023-10-20 11:57 ` Abdellatif El Khlifi
1 sibling, 1 reply; 36+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-24 9:27 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, nd
Hi Tom,
> Here's the latest report
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Aug 21, 2023 at 4:30 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.
>
> 4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 4 of 4 defect(s)
>
>
> ** CID 464361: Control flow issues (DEADCODE)
> /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
Well received, I started working on that.
I'll provide a fix after coming back fom holidays (mid September)
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2023-08-21 21:09 Tom Rini
2023-08-24 9:27 ` Abdellatif El Khlifi
2023-10-20 11:57 ` Abdellatif El Khlifi
0 siblings, 2 replies; 36+ messages in thread
From: Tom Rini @ 2023-08-21 21:09 UTC (permalink / raw)
To: u-boot, Alvaro Fernando García, Abdellatif El Khlifi
[-- Attachment #1: Type: text/plain, Size: 5339 bytes --]
Here's the latest report
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Aug 21, 2023 at 4:30 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.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)
** CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/drivers/video/pwm_backlight.c: 68 in set_pwm()
________________________________________________________________________________________________________
*** CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/drivers/video/pwm_backlight.c: 68 in set_pwm()
62 {
63 u64 width;
64 uint duty_cycle;
65 int ret;
66
67 if (priv->period_ns) {
>>> CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "priv->period_ns * (priv->cur_level - priv->min_level)" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
68 width = priv->period_ns * (priv->cur_level - priv->min_level);
69 duty_cycle = div_u64(width,
70 (priv->max_level - priv->min_level));
71 ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
72 duty_cycle);
73 } else {
** CID 464361: Control flow issues (DEADCODE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
________________________________________________________________________________________________________
*** CID 464361: Control flow issues (DEADCODE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
142
143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID)
144 return -EINVAL;
145
146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id);
147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
>>> CID 464361: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return -22;".
148 return -EINVAL;
149
150 if (!err_msg_map[abi_idx].err_str[err_idx])
151 return -EINVAL;
152
153 log_err("%s\n", err_msg_map[abi_idx].err_str[err_idx]);
** CID 464360: Control flow issues (NO_EFFECT)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
________________________________________________________________________________________________________
*** CID 464360: Control flow issues (NO_EFFECT)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
201 major = GET_FFA_MAJOR_VERSION(res.a0);
202 minor = GET_FFA_MINOR_VERSION(res.a0);
203
204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n",
205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor);
206
>>> CID 464360: Control flow issues (NO_EFFECT)
>>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".
207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
208 log_debug("FF-A versions are compatible\n");
209
210 if (dev) {
211 uc_priv = dev_get_uclass_priv(dev);
212 if (uc_priv)
** CID 464359: (PASS_BY_VALUE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
/drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
________________________________________________________________________________________________________
*** CID 464359: (PASS_BY_VALUE)
/drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn()
162 * @args: FF-A ABI arguments to be copied to Xn registers
163 * @res: FF-A ABI return data to be copied from Xn registers
164 *
165 * Calls low level SMC implementation.
166 * This function should be implemented by the user driver.
167 */
>>> CID 464359: (PASS_BY_VALUE)
>>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
169 {
170 }
171
172 /**
173 * ffa_get_version_hdlr() - FFA_VERSION handler function
/drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
667 * invoke_ffa_fn() - SMC wrapper
668 * @args: FF-A ABI arguments to be copied to Xn registers
669 * @res: FF-A ABI return data to be copied from Xn registers
670 *
671 * Calls the emulated SMC call.
672 */
>>> CID 464359: (PASS_BY_VALUE)
>>> Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
674 {
675 sandbox_arm_ffa_smccc_smc(&args, res);
676 }
677
678 /**
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-05-08 20:20 Tom Rini
2023-05-15 21:59 ` Ehsan Mohandesi
@ 2023-05-18 21:04 ` Sean Edmond
1 sibling, 0 replies; 36+ messages in thread
From: Sean Edmond @ 2023-05-18 21:04 UTC (permalink / raw)
To: Tom Rini, u-boot; +Cc: Sean Edmond
On 2023-05-08 1:20 p.m., Tom Rini wrote:
> Here's the latest defect report:
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, May 8, 2023, 2:29 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.
>
> 5 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 5 of 5 defect(s)
>
>
> ** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
> /cmd/net.c: 279 in netboot_update_env()
>
>
> ________________________________________________________________________________________________________
> *** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
> /cmd/net.c: 279 in netboot_update_env()
> 273
> 274 if (IS_ENABLED(CONFIG_IPV6)) {
> 275 if (!ip6_is_unspecified_addr(&net_ip6) ||
> 276 net_prefix_length != 0) {
> 277 sprintf(tmp, "%pI6c", &net_ip6);
> 278 if (net_prefix_length != 0)
>>>> CID 453851: Memory - corruptions (OVERLAPPING_COPY)
>>>> In the call to function "sprintf", the arguments "tmp" and "tmp"
Just submitted a patch to fix 453851.
> may point to the same object.
> 279 sprintf(tmp, "%s/%d", tmp,
> net_prefix_length);
> 280
> 281 env_set("ip6addr", tmp);
> 282 }
> 283
> 284 if (!ip6_is_unspecified_addr(&net_server_ip6)) {
>
> ** CID 450971: Insecure data handling (TAINTED_SCALAR)
> /net/ndisc.c: 391 in process_ra()
>
>
> ________________________________________________________________________________________________________
> *** CID 450971: Insecure data handling (TAINTED_SCALAR)
> /net/ndisc.c: 391 in process_ra()
> 385 /* Ignore the packet if router lifetime is 0. */
> 386 if (!icmp->icmp6_rt_lifetime)
> 387 return -EOPNOTSUPP;
> 388
> 389 /* Processing the options */
> 390 option = msg->opt;
>>>> CID 450971: Insecure data handling (TAINTED_SCALAR)
>>>> Using tainted variable "remaining_option_len" as a loop boundary.
> 391 while (remaining_option_len > 0) {
> 392 /* The 2nd byte of the option is its length. */
> 393 option_len = option[1];
> 394 /* All included options should have a positive
> length. */
> 395 if (option_len == 0)
> 396 return -EINVAL;
>
> ** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
> /net/ndisc.c: 209 in ip6_send_rs()
>
>
> ________________________________________________________________________________________________________
> *** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
> /net/ndisc.c: 209 in ip6_send_rs()
> 203 icmp_len, PROT_ICMPV6, pcsum);
> 204 msg->icmph.icmp6_cksum = csum;
> 205 pkt += icmp_len;
> 206
> 207 /* Wait up to 1 second if it is the first try to get the RA
> */
> 208 if (retry_count == 0)
>>>> CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 209 udelay(((unsigned int)rand() % 1000000) *
> MAX_SOLICITATION_DELAY);
> 210
> 211 /* send it! */
> 212 net_send_packet(net_tx_packet, (pkt - net_tx_packet));
> 213
> 214 retry_count++;
>
> ** CID 436282: (DC.WEAK_CRYPTO)
> /net/dhcpv6.c: 621 in dhcp6_state_machine()
> /net/dhcpv6.c: 627 in dhcp6_state_machine()
> /net/dhcpv6.c: 628 in dhcp6_state_machine()
> /net/dhcpv6.c: 662 in dhcp6_state_machine()
> /net/dhcpv6.c: 613 in dhcp6_state_machine()
>
>
> ________________________________________________________________________________________________________
> *** CID 436282: (DC.WEAK_CRYPTO)
> /net/dhcpv6.c: 621 in dhcp6_state_machine()
> 615 /* handle state machine entry conditions */
> 616 if (sm_params.curr_state != sm_params.next_state) {
> 617 sm_params.retry_cnt = 0;
> 618
> 619 if (sm_params.next_state == DHCP6_SOLICIT) {
> 620 /* delay a random ammount (special for
> SOLICIT) */
>>>> CID 436282: (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 621 udelay((rand() % SOL_MAX_DELAY_MS) * 1000);
> 622 /* init timestamp variables after SOLICIT
> delay */
> 623 sm_params.dhcp6_start_ms = get_timer(0);
> 624 sm_params.dhcp6_retry_start_ms =
> sm_params.dhcp6_start_ms;
> 625 sm_params.dhcp6_retry_ms =
> sm_params.dhcp6_start_ms;
> 626 /* init transaction and ia_id */
> /net/dhcpv6.c: 627 in dhcp6_state_machine()
> 621 udelay((rand() % SOL_MAX_DELAY_MS) * 1000);
> 622 /* init timestamp variables after SOLICIT
> delay */
> 623 sm_params.dhcp6_start_ms = get_timer(0);
> 624 sm_params.dhcp6_retry_start_ms =
> sm_params.dhcp6_start_ms;
> 625 sm_params.dhcp6_retry_ms =
> sm_params.dhcp6_start_ms;
> 626 /* init transaction and ia_id */
>>>> CID 436282: (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 627 sm_params.trans_id = rand() & 0xFFFFFF;
> 628 sm_params.ia_id = rand();
> 629 /* initialize retransmission parameters */
> 630 sm_params.irt_ms = SOL_TIMEOUT_MS;
> 631 sm_params.mrt_ms = updated_sol_max_rt_ms;
> 632 /* RFCs default MRC is be 0 (try infinitely)
> /net/dhcpv6.c: 628 in dhcp6_state_machine()
> 622 /* init timestamp variables after SOLICIT
> delay */
> 623 sm_params.dhcp6_start_ms = get_timer(0);
> 624 sm_params.dhcp6_retry_start_ms =
> sm_params.dhcp6_start_ms;
> 625 sm_params.dhcp6_retry_ms =
> sm_params.dhcp6_start_ms;
> 626 /* init transaction and ia_id */
> 627 sm_params.trans_id = rand() & 0xFFFFFF;
>>>> CID 436282: (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 628 sm_params.ia_id = rand();
> 629 /* initialize retransmission parameters */
> 630 sm_params.irt_ms = SOL_TIMEOUT_MS;
> 631 sm_params.mrt_ms = updated_sol_max_rt_ms;
> 632 /* RFCs default MRC is be 0 (try infinitely)
> 633 * give up after CONFIG_NET_RETRY_COUNT
> number of tries (same as DHCPv4)
> /net/dhcpv6.c: 662 in dhcp6_state_machine()
> 656 (sm_params.mrd_ms != 0 &&
> 657 ((sm_params.dhcp6_retry_ms -
> sm_params.dhcp6_retry_start_ms) >= sm_params.mrd_ms))) {
> 658 sm_params.next_state = DHCP6_FAIL;
> 659 }
> 660
> 661 /* calculate retransmission timeout (RT) */
>>>> CID 436282: (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 662 rand_minus_plus_100 = ((rand() % 200) - 100);
> 663 if (sm_params.retry_cnt == 0) {
> 664 sm_params.rt_ms = sm_params.irt_ms +
> 665 ((sm_params.irt_ms *
> rand_minus_plus_100) / 1000);
> 666 } else {
> 667 sm_params.rt_ms = (2 * sm_params.rt_prev_ms) +
> /net/dhcpv6.c: 613 in dhcp6_state_machine()
> 607 * Proceed anyway to proceed DONE/FAIL actions
> 608 */
> 609 debug("Unexpected DHCP6 state : %d\n",
> sm_params.curr_state);
> 610 break;
> 611 }
> 612 /* re-seed the RNG */
>>>> CID 436282: (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
We can ignore 436282. For DHCP6, we seed using srand_mac() to ensure
that rand() will produce enough variation for each device on the
network. The numbers from rand() are used to introduce variation in the
delay between re-transmissions to avoid excessive server congestion if
all clients are started at the same time (such as a power loss). These
values are not used for crypto.
> because linear congruential algorithms are too easy to break.
> 613 srand(get_ticks() + rand());
> 614
> 615 /* handle state machine entry conditions */
> 616 if (sm_params.curr_state != sm_params.next_state) {
> 617 sm_params.retry_cnt = 0;
> 618
>
> ** CID 436278: (TAINTED_SCALAR)
> /net/dhcpv6.c: 321 in dhcp6_parse_options()
>
>
> ________________________________________________________________________________________________________
> *** CID 436278: (TAINTED_SCALAR)
> /net/dhcpv6.c: 376 in dhcp6_parse_options()
> 370 if (sm_params.curr_state ==
> DHCP6_SOLICIT)
> 371 sm_params.mrt_ms =
> updated_sol_max_rt_ms;
> 372 }
> 373 break;
> 374 case DHCP6_OPTION_OPT_BOOTFILE_URL:
> 375 debug("DHCP6_OPTION_OPT_BOOTFILE_URL
> FOUND\n");
>>>> CID 436278: (TAINTED_SCALAR)
>>>> Passing tainted expression "option_len + 1" to "copy_filename",
> which uses it as a loop boundary.
> 376 copy_filename(net_boot_file_name,
> option_ptr, option_len + 1);
> 377 debug("net_boot_file_name: %s\n",
> net_boot_file_name);
> 378
> 379 /* copy server_ip6 (required for PXE) */
> 380 s = strchr(net_boot_file_name, '[');
> 381 e = strchr(net_boot_file_name, ']');
> /net/dhcpv6.c: 321 in dhcp6_parse_options()
> 315 while (option_hdr < (struct dhcp6_option_hdr *)(rx_pkt +
> len)) {
> 316 option_ptr = ((uchar *)option_hdr) + sizeof(struct
> dhcp6_hdr);
> 317 option_len = ntohs(option_hdr->option_len);
> 318
> 319 switch (ntohs(option_hdr->option_id)) {
> 320 case DHCP6_OPTION_CLIENTID:
>>>> CID 436278: (TAINTED_SCALAR)
>>>> Passing tainted expression "option_len" to "memcmp", which uses it
Just submitted a patch to fix 436278.
> as an offset. [Note: The source code implementation of the function has
> been overridden by a builtin model.]
> 321 if (memcmp(option_ptr, sm_params.duid,
> option_len)
> 322 != 0) {
> 323 debug("CLIENT ID DOESN'T MATCH\n");
> 324 } else {
> 325 debug("CLIENT ID FOUND and
> MATCHES\n");
> 326 sm_params.rx_status.client_id_match
> = true;
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
2023-05-08 20:20 Tom Rini
@ 2023-05-15 21:59 ` Ehsan Mohandesi
2023-05-18 21:04 ` Sean Edmond
1 sibling, 0 replies; 36+ messages in thread
From: Ehsan Mohandesi @ 2023-05-15 21:59 UTC (permalink / raw)
To: Tom Rini, u-boot; +Cc: Sean Edmond
On 5/8/2023 3:20 PM, Tom Rini wrote:
> Here's the latest defect report:
>
> ---------- Forwarded message ---------
> From:<scan-admin@coverity.com>
> Date: Mon, May 8, 2023, 2:29 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.
>
> 5 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 5 of 5 defect(s)
>
>
> ** CID 450971: Insecure data handling (TAINTED_SCALAR)
> /net/ndisc.c: 391 in process_ra()
>
>
> ________________________________________________________________________________________________________
> *** CID 450971: Insecure data handling (TAINTED_SCALAR)
> /net/ndisc.c: 391 in process_ra()
> 385 /* Ignore the packet if router lifetime is 0. */
> 386 if (!icmp->icmp6_rt_lifetime)
> 387 return -EOPNOTSUPP;
> 388
> 389 /* Processing the options */
> 390 option = msg->opt;
>>>> CID 450971: Insecure data handling (TAINTED_SCALAR)
>>>> Using tainted variable "remaining_option_len" as a loop boundary.
> 391 while (remaining_option_len > 0) {
> 392 /* The 2nd byte of the option is its length. */
> 393 option_len = option[1];
> 394 /* All included options should have a positive
> length. */
> 395 if (option_len == 0)
> 396 return -EINVAL;
The problem here is that although the lower bound of the variable
remaining_option_len is checked, the upper bound is not checked.
Coverity is complaining that the function's argument len which is read
from a packet content is assigned to remaining_option_len and therefore
has made it a tainted scalar.
I will compare the value of len with ETH_MAX_MTU constant and make sure
it is less than that as shown below.
if(len > ETH_MAX_MTU) return-EMSGSIZE;
> ** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
> /net/ndisc.c: 209 in ip6_send_rs()
>
>
> ________________________________________________________________________________________________________
> *** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
> /net/ndisc.c: 209 in ip6_send_rs()
> 203 icmp_len, PROT_ICMPV6, pcsum);
> 204 msg->icmph.icmp6_cksum = csum;
> 205 pkt += icmp_len;
> 206
> 207 /* Wait up to 1 second if it is the first try to get the RA
> */
> 208 if (retry_count == 0)
>>>> CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
>>>> "rand" should not be used for security-related applications,
> because linear congruential algorithms are too easy to break.
> 209 udelay(((unsigned int)rand() % 1000000) *
> MAX_SOLICITATION_DELAY);
> 210
> 211 /* send it! */
> 212 net_send_packet(net_tx_packet, (pkt - net_tx_packet));
> 213
> 214 retry_count++;
This is a false positive. The function rand() is not used for encryption
here. It is used to just make a random delay to avoid collisions on the
network. It has nothing to do with encryption.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2023-05-08 20:20 Tom Rini
2023-05-15 21:59 ` Ehsan Mohandesi
2023-05-18 21:04 ` Sean Edmond
0 siblings, 2 replies; 36+ messages in thread
From: Tom Rini @ 2023-05-08 20:20 UTC (permalink / raw)
To: u-boot; +Cc: Sean Edmond
[-- Attachment #1: Type: text/plain, Size: 11377 bytes --]
Here's the latest defect report:
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, May 8, 2023, 2:29 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.
5 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 5 of 5 defect(s)
** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
/cmd/net.c: 279 in netboot_update_env()
________________________________________________________________________________________________________
*** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
/cmd/net.c: 279 in netboot_update_env()
273
274 if (IS_ENABLED(CONFIG_IPV6)) {
275 if (!ip6_is_unspecified_addr(&net_ip6) ||
276 net_prefix_length != 0) {
277 sprintf(tmp, "%pI6c", &net_ip6);
278 if (net_prefix_length != 0)
>>> CID 453851: Memory - corruptions (OVERLAPPING_COPY)
>>> In the call to function "sprintf", the arguments "tmp" and "tmp"
may point to the same object.
279 sprintf(tmp, "%s/%d", tmp,
net_prefix_length);
280
281 env_set("ip6addr", tmp);
282 }
283
284 if (!ip6_is_unspecified_addr(&net_server_ip6)) {
** CID 450971: Insecure data handling (TAINTED_SCALAR)
/net/ndisc.c: 391 in process_ra()
________________________________________________________________________________________________________
*** CID 450971: Insecure data handling (TAINTED_SCALAR)
/net/ndisc.c: 391 in process_ra()
385 /* Ignore the packet if router lifetime is 0. */
386 if (!icmp->icmp6_rt_lifetime)
387 return -EOPNOTSUPP;
388
389 /* Processing the options */
390 option = msg->opt;
>>> CID 450971: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "remaining_option_len" as a loop boundary.
391 while (remaining_option_len > 0) {
392 /* The 2nd byte of the option is its length. */
393 option_len = option[1];
394 /* All included options should have a positive
length. */
395 if (option_len == 0)
396 return -EINVAL;
** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
/net/ndisc.c: 209 in ip6_send_rs()
________________________________________________________________________________________________________
*** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
/net/ndisc.c: 209 in ip6_send_rs()
203 icmp_len, PROT_ICMPV6, pcsum);
204 msg->icmph.icmp6_cksum = csum;
205 pkt += icmp_len;
206
207 /* Wait up to 1 second if it is the first try to get the RA
*/
208 if (retry_count == 0)
>>> CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
209 udelay(((unsigned int)rand() % 1000000) *
MAX_SOLICITATION_DELAY);
210
211 /* send it! */
212 net_send_packet(net_tx_packet, (pkt - net_tx_packet));
213
214 retry_count++;
** CID 436282: (DC.WEAK_CRYPTO)
/net/dhcpv6.c: 621 in dhcp6_state_machine()
/net/dhcpv6.c: 627 in dhcp6_state_machine()
/net/dhcpv6.c: 628 in dhcp6_state_machine()
/net/dhcpv6.c: 662 in dhcp6_state_machine()
/net/dhcpv6.c: 613 in dhcp6_state_machine()
________________________________________________________________________________________________________
*** CID 436282: (DC.WEAK_CRYPTO)
/net/dhcpv6.c: 621 in dhcp6_state_machine()
615 /* handle state machine entry conditions */
616 if (sm_params.curr_state != sm_params.next_state) {
617 sm_params.retry_cnt = 0;
618
619 if (sm_params.next_state == DHCP6_SOLICIT) {
620 /* delay a random ammount (special for
SOLICIT) */
>>> CID 436282: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
621 udelay((rand() % SOL_MAX_DELAY_MS) * 1000);
622 /* init timestamp variables after SOLICIT
delay */
623 sm_params.dhcp6_start_ms = get_timer(0);
624 sm_params.dhcp6_retry_start_ms =
sm_params.dhcp6_start_ms;
625 sm_params.dhcp6_retry_ms =
sm_params.dhcp6_start_ms;
626 /* init transaction and ia_id */
/net/dhcpv6.c: 627 in dhcp6_state_machine()
621 udelay((rand() % SOL_MAX_DELAY_MS) * 1000);
622 /* init timestamp variables after SOLICIT
delay */
623 sm_params.dhcp6_start_ms = get_timer(0);
624 sm_params.dhcp6_retry_start_ms =
sm_params.dhcp6_start_ms;
625 sm_params.dhcp6_retry_ms =
sm_params.dhcp6_start_ms;
626 /* init transaction and ia_id */
>>> CID 436282: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
627 sm_params.trans_id = rand() & 0xFFFFFF;
628 sm_params.ia_id = rand();
629 /* initialize retransmission parameters */
630 sm_params.irt_ms = SOL_TIMEOUT_MS;
631 sm_params.mrt_ms = updated_sol_max_rt_ms;
632 /* RFCs default MRC is be 0 (try infinitely)
/net/dhcpv6.c: 628 in dhcp6_state_machine()
622 /* init timestamp variables after SOLICIT
delay */
623 sm_params.dhcp6_start_ms = get_timer(0);
624 sm_params.dhcp6_retry_start_ms =
sm_params.dhcp6_start_ms;
625 sm_params.dhcp6_retry_ms =
sm_params.dhcp6_start_ms;
626 /* init transaction and ia_id */
627 sm_params.trans_id = rand() & 0xFFFFFF;
>>> CID 436282: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
628 sm_params.ia_id = rand();
629 /* initialize retransmission parameters */
630 sm_params.irt_ms = SOL_TIMEOUT_MS;
631 sm_params.mrt_ms = updated_sol_max_rt_ms;
632 /* RFCs default MRC is be 0 (try infinitely)
633 * give up after CONFIG_NET_RETRY_COUNT
number of tries (same as DHCPv4)
/net/dhcpv6.c: 662 in dhcp6_state_machine()
656 (sm_params.mrd_ms != 0 &&
657 ((sm_params.dhcp6_retry_ms -
sm_params.dhcp6_retry_start_ms) >= sm_params.mrd_ms))) {
658 sm_params.next_state = DHCP6_FAIL;
659 }
660
661 /* calculate retransmission timeout (RT) */
>>> CID 436282: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
662 rand_minus_plus_100 = ((rand() % 200) - 100);
663 if (sm_params.retry_cnt == 0) {
664 sm_params.rt_ms = sm_params.irt_ms +
665 ((sm_params.irt_ms *
rand_minus_plus_100) / 1000);
666 } else {
667 sm_params.rt_ms = (2 * sm_params.rt_prev_ms) +
/net/dhcpv6.c: 613 in dhcp6_state_machine()
607 * Proceed anyway to proceed DONE/FAIL actions
608 */
609 debug("Unexpected DHCP6 state : %d\n",
sm_params.curr_state);
610 break;
611 }
612 /* re-seed the RNG */
>>> CID 436282: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
613 srand(get_ticks() + rand());
614
615 /* handle state machine entry conditions */
616 if (sm_params.curr_state != sm_params.next_state) {
617 sm_params.retry_cnt = 0;
618
** CID 436278: (TAINTED_SCALAR)
/net/dhcpv6.c: 321 in dhcp6_parse_options()
________________________________________________________________________________________________________
*** CID 436278: (TAINTED_SCALAR)
/net/dhcpv6.c: 376 in dhcp6_parse_options()
370 if (sm_params.curr_state ==
DHCP6_SOLICIT)
371 sm_params.mrt_ms =
updated_sol_max_rt_ms;
372 }
373 break;
374 case DHCP6_OPTION_OPT_BOOTFILE_URL:
375 debug("DHCP6_OPTION_OPT_BOOTFILE_URL
FOUND\n");
>>> CID 436278: (TAINTED_SCALAR)
>>> Passing tainted expression "option_len + 1" to "copy_filename",
which uses it as a loop boundary.
376 copy_filename(net_boot_file_name,
option_ptr, option_len + 1);
377 debug("net_boot_file_name: %s\n",
net_boot_file_name);
378
379 /* copy server_ip6 (required for PXE) */
380 s = strchr(net_boot_file_name, '[');
381 e = strchr(net_boot_file_name, ']');
/net/dhcpv6.c: 321 in dhcp6_parse_options()
315 while (option_hdr < (struct dhcp6_option_hdr *)(rx_pkt +
len)) {
316 option_ptr = ((uchar *)option_hdr) + sizeof(struct
dhcp6_hdr);
317 option_len = ntohs(option_hdr->option_len);
318
319 switch (ntohs(option_hdr->option_id)) {
320 case DHCP6_OPTION_CLIENTID:
>>> CID 436278: (TAINTED_SCALAR)
>>> Passing tainted expression "option_len" to "memcmp", which uses it
as an offset. [Note: The source code implementation of the function has
been overridden by a builtin model.]
321 if (memcmp(option_ptr, sm_params.duid,
option_len)
322 != 0) {
323 debug("CLIENT ID DOESN'T MATCH\n");
324 } else {
325 debug("CLIENT ID FOUND and
MATCHES\n");
326 sm_params.rx_status.client_id_match
= true;
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2023-02-14 14:26 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2023-02-14 14:26 UTC (permalink / raw)
To: u-boot
[-- Attachment #1: Type: text/plain, Size: 2245 bytes --]
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Feb 13, 2023, 6:50 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.
3 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 436073: Resource leaks (RESOURCE_LEAK)
/tools/proftool.c: 1853 in make_flamegraph()
________________________________________________________________________________________________________
*** CID 436073: Resource leaks (RESOURCE_LEAK)
/tools/proftool.c: 1853 in make_flamegraph()
1847
1848 if (make_flame_tree(out_format, &tree))
1849 return -1;
1850
1851 *str = '\0';
1852 if (output_tree(fout, out_format, tree, str, sizeof(str),
0))
>>> CID 436073: Resource leaks (RESOURCE_LEAK)
>>> Variable "tree" going out of scope leaks the storage it points to.
1853 return -1;
1854
1855 return 0;
1856 }
1857
1858 /**
** CID 436072: Insecure data handling (TAINTED_SCALAR)
________________________________________________________________________________________________________
*** CID 436072: Insecure data handling (TAINTED_SCALAR)
/tools/proftool.c: 515 in read_trace()
509 switch (hdr.type) {
510 case TRACE_CHUNK_FUNCS:
511 /* Ignored at present */
512 break;
513
514 case TRACE_CHUNK_CALLS:
>>> CID 436072: Insecure data handling (TAINTED_SCALAR)
>>> Passing tainted expression "hdr.rec_count" to "read_calls", which
uses it as an allocation size.
515 if (read_calls(fin, hdr.rec_count))
516 return 1;
517 break;
518 }
519 }
520 return 0;
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2022-11-21 19:43 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2022-11-21 19:43 UTC (permalink / raw)
To: u-boot; +Cc: Simon Glass
[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]
Here's the latest report
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Nov 21, 2022 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.
2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
3 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 376996: Error handling issues (CHECKED_RETURN)
/drivers/net/sandbox-raw-bus.c: 40 in eth_raw_bus_post_bind()
________________________________________________________________________________________________________
*** CID 376996: Error handling issues (CHECKED_RETURN)
/drivers/net/sandbox-raw-bus.c: 40 in eth_raw_bus_post_bind()
34 if (skip_localhost && local)
35 continue;
36
37 ub_ifname = calloc(IFNAMSIZ + sizeof(ub_ifname_pfx), 1);
38 strcpy(ub_ifname, ub_ifname_pfx);
39 strncat(ub_ifname, i->if_name, IFNAMSIZ);
>>> CID 376996: Error handling issues (CHECKED_RETURN)
>>> Calling "device_bind_driver" without checking return value (as is done elsewhere 12 out of 15 times).
40 device_bind_driver(dev, "eth_sandbox_raw", ub_ifname, &child);
41
42 device_set_name_alloced(child);
43 device_probe(child);
44 priv = dev_get_priv(child);
45 if (priv) {
** CID 376995: Null pointer dereferences (FORWARD_NULL)
/test/test-main.c: 518 in ut_run_tests()
________________________________________________________________________________________________________
*** CID 376995: Null pointer dereferences (FORWARD_NULL)
/test/test-main.c: 518 in ut_run_tests()
512 pos = dectoul(test_insert, NULL);
513 p = strchr(test_insert, ':');
514 if (p)
515 p++;
516
517 for (test = tests; test < tests + count; test++) {
>>> CID 376995: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "p" to "strcmp", which dereferences it. [Note: The source code implementation of the function has been overridden by a builtin model.]
518 if (!strcmp(p, test->name))
519 one = test;
520 }
521 }
522
523 for (upto = 0, test = tests; test < tests + count;
test++, upto++) {
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2022-11-09 15:40 Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2022-11-09 15:40 UTC (permalink / raw)
To: u-boot; +Cc: Simon Glass, Heinrich Schuchardt
[-- Attachment #1: Type: text/plain, Size: 32083 bytes --]
Here's the latest report.
---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Nov 7, 2022 at 3: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.
21 new defect(s) introduced to Das U-Boot found with Coverity Scan.
15 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 20 of 21 defect(s)
** CID 376213: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2642 in
efi_install_multiple_protocol_interfaces_int()
________________________________________________________________________________________________________
*** CID 376213: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2642 in
efi_install_multiple_protocol_interfaces_int()
2636 int i = 0;
2637 efi_va_list argptr_copy;
2638
2639 if (!handle)
2640 return EFI_INVALID_PARAMETER;
2641
>>> CID 376213: Memory - illegal accesses (UNINIT)
>>> Using uninitialized value "argptr_copy" when calling "__builtin_ms_va_copy".
2642 efi_va_copy(argptr_copy, argptr);
2643 for (;;) {
2644 protocol = efi_va_arg(argptr, efi_guid_t*);
2645 if (!protocol)
2646 break;
2647 protocol_interface = efi_va_arg(argptr, void*);
** CID 376212: Error handling issues (CHECKED_RETURN)
________________________________________________________________________________________________________
*** CID 376212: Error handling issues (CHECKED_RETURN)
/drivers/usb/emul/sandbox_flash.c: 197 in handle_ufi_command()
191
192 ret = sb_scsi_emul_command(info, req, len);
193 if (!ret) {
194 setup_response(priv);
195 } else if ((ret == SCSI_EMUL_DO_READ || ret ==
SCSI_EMUL_DO_WRITE) &&
196 priv->fd != -1) {
>>> CID 376212: Error handling issues (CHECKED_RETURN)
>>> Calling "os_lseek(priv->fd, info->seek_block * info->block_size, 0)" without checking return value. It wraps a library function that may fail and return an error code.
197 os_lseek(priv->fd, info->seek_block * info->block_size,
198 OS_SEEK_SET);
199 setup_response(priv);
200 } else {
201 setup_fail_response(priv);
202 }
** CID 376211: (TAINTED_SCALAR)
________________________________________________________________________________________________________
*** CID 376211: (TAINTED_SCALAR)
/cmd/eficonfig.c: 1475 in eficonfig_edit_boot_option()
1469 if (lo.file_path)
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "initrd_dp->length" to "fill_file_info", which uses it as an offset.
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
1476 efi_free_pool(initrd_dp);
1477 }
1478
1479 if (size > 0)
1480 memcpy(bo->optional_data,
lo.optional_data, size);
/cmd/eficonfig.c: 1535 in eficonfig_edit_boot_option()
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
1534 free(bo->optional_data);
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "*bo->description" to "dlfree", which uses it as an offset.
1535 free(bo->description);
1536 free(bo->file_info.current_path);
1537 free(bo->initrd_info.current_path);
1538 efi_free_pool(device_dp);
1539 efi_free_pool(initrd_device_dp);
1540 efi_free_pool(initrd_dp);
/cmd/eficonfig.c: 1534 in eficonfig_edit_boot_option()
1528
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "*bo->optional_data" to "dlfree", which uses it as an offset.
1534 free(bo->optional_data);
1535 free(bo->description);
1536 free(bo->file_info.current_path);
1537 free(bo->initrd_info.current_path);
1538 efi_free_pool(device_dp);
1539 efi_free_pool(initrd_device_dp);
/cmd/eficonfig.c: 1534 in eficonfig_edit_boot_option()
1528
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "*bo->optional_data" to "dlfree", which uses it as an offset.
1534 free(bo->optional_data);
1535 free(bo->description);
1536 free(bo->file_info.current_path);
1537 free(bo->initrd_info.current_path);
1538 efi_free_pool(device_dp);
1539 efi_free_pool(initrd_device_dp);
/cmd/eficonfig.c: 1535 in eficonfig_edit_boot_option()
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
1534 free(bo->optional_data);
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "*bo->description" to "dlfree", which uses it as an offset.
1535 free(bo->description);
1536 free(bo->file_info.current_path);
1537 free(bo->initrd_info.current_path);
1538 efi_free_pool(device_dp);
1539 efi_free_pool(initrd_device_dp);
1540 efi_free_pool(initrd_dp);
/cmd/eficonfig.c: 1535 in eficonfig_edit_boot_option()
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
1534 free(bo->optional_data);
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "*bo->description" to "dlfree", which uses it as an offset.
1535 free(bo->description);
1536 free(bo->file_info.current_path);
1537 free(bo->initrd_info.current_path);
1538 efi_free_pool(device_dp);
1539 efi_free_pool(initrd_device_dp);
1540 efi_free_pool(initrd_dp);
/cmd/eficonfig.c: 1473 in eficonfig_edit_boot_option()
1467
1468 /* EFI image file path is a first instance */
1469 if (lo.file_path)
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "lo.file_path" to "efi_dp_from_lo", which uses it as a loop boundary.
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
1476 efi_free_pool(initrd_dp);
1477 }
1478
/cmd/eficonfig.c: 1475 in eficonfig_edit_boot_option()
1469 if (lo.file_path)
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "initrd_dp->str" to "fill_file_info", which uses it as an offset.
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
1476 efi_free_pool(initrd_dp);
1477 }
1478
1479 if (size > 0)
1480 memcpy(bo->optional_data,
lo.optional_data, size);
/cmd/eficonfig.c: 1473 in eficonfig_edit_boot_option()
1467
1468 /* EFI image file path is a first instance */
1469 if (lo.file_path)
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "lo.file_path_length" to "efi_dp_from_lo", which uses it as a loop boundary.
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
1476 efi_free_pool(initrd_dp);
1477 }
1478
/cmd/eficonfig.c: 1470 in eficonfig_edit_boot_option()
1464 lo.label[EFICONFIG_DESCRIPTION_MAX - 1] = u'\0';
1465
1466 u16_strcpy(bo->description, lo.label);
1467
1468 /* EFI image file path is a first instance */
1469 if (lo.file_path)
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "lo.file_path->str" to "fill_file_info", which uses it as an offset.
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
/cmd/eficonfig.c: 1470 in eficonfig_edit_boot_option()
1464 lo.label[EFICONFIG_DESCRIPTION_MAX - 1] = u'\0';
1465
1466 u16_strcpy(bo->description, lo.label);
1467
1468 /* EFI image file path is a first instance */
1469 if (lo.file_path)
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "lo.file_path->length" to "fill_file_info", which uses it as an offset.
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
/cmd/eficonfig.c: 1473 in eficonfig_edit_boot_option()
1467
1468 /* EFI image file path is a first instance */
1469 if (lo.file_path)
1470 fill_file_info(lo.file_path,
&bo->file_info, device_dp);
1471
1472 /* Initrd file path(optional) is placed at
second instance. */
>>> CID 376211: (TAINTED_SCALAR)
>>> Passing tainted expression "lo.file_path_length" to "efi_dp_from_lo", which uses it as a loop boundary.
1473 initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid);
1474 if (initrd_dp) {
1475 fill_file_info(initrd_dp,
&bo->initrd_info, initrd_device_dp);
1476 efi_free_pool(initrd_dp);
1477 }
1478
** CID 376210: (BUFFER_SIZE)
/drivers/scsi/scsi_emul.c: 35 in sb_scsi_emul_command()
/drivers/scsi/scsi_emul.c: 36 in sb_scsi_emul_command()
________________________________________________________________________________________________________
*** CID 376210: (BUFFER_SIZE)
/drivers/scsi/scsi_emul.c: 35 in sb_scsi_emul_command()
29 struct scsi_inquiry_resp *resp = (void *)info->buff;
30
31 info->alloc_len = req->cmd[4];
32 memset(resp, '\0', sizeof(*resp));
33 resp->data_format = 1;
34 resp->additional_len = 0x1f;
>>> CID 376210: (BUFFER_SIZE)
>>> Calling "strncpy" with a maximum size argument of 8 bytes on destination array "resp->vendor" of size 8 bytes might leave the destination string unterminated.
35 strncpy(resp->vendor, info->vendor, sizeof(resp->vendor));
36 strncpy(resp->product, info->product, sizeof(resp->product));
37 strncpy(resp->revision, "1.0", sizeof(resp->revision));
38 info->buff_used = sizeof(*resp);
39 break;
40 }
/drivers/scsi/scsi_emul.c: 36 in sb_scsi_emul_command()
30
31 info->alloc_len = req->cmd[4];
32 memset(resp, '\0', sizeof(*resp));
33 resp->data_format = 1;
34 resp->additional_len = 0x1f;
35 strncpy(resp->vendor, info->vendor, sizeof(resp->vendor));
>>> CID 376210: (BUFFER_SIZE)
>>> Calling "strncpy" with a maximum size argument of 16 bytes on destination array "resp->product" of size 16 bytes might leave the destination string unterminated.
36 strncpy(resp->product, info->product, sizeof(resp->product));
37 strncpy(resp->revision, "1.0", sizeof(resp->revision));
38 info->buff_used = sizeof(*resp);
39 break;
40 }
41 case SCSI_TST_U_RDY:
** CID 376209: Null pointer dereferences (REVERSE_INULL)
/drivers/pci/pci-uclass.c: 1249 in pci_find_next_device()
________________________________________________________________________________________________________
*** CID 376209: Null pointer dereferences (REVERSE_INULL)
/drivers/pci/pci-uclass.c: 1249 in pci_find_next_device()
1243 }
1244 }
1245
1246 /* We ran out of siblings. Try the next bus */
1247 uclass_next_device(&bus);
1248
>>> CID 376209: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "bus" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
1249 return bus ? skip_to_next_device(bus, devp) : 0;
1250 }
1251
1252 int pci_find_first_device(struct udevice **devp)
1253 {
1254 struct udevice *bus;
** CID 376208: Null pointer dereferences (REVERSE_INULL)
/cmd/virtio.c: 31 in do_virtio()
________________________________________________________________________________________________________
*** CID 376208: Null pointer dereferences (REVERSE_INULL)
/cmd/virtio.c: 31 in do_virtio()
25 struct udevice *bus, *child;
26
27 uclass_first_device(UCLASS_VIRTIO, &bus);
28 if (!bus)
29 return CMD_RET_FAILURE;
30
>>> CID 376208: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "bus" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
31 while (bus) {
32 device_foreach_child_probe(child, bus)
33 ;
34 uclass_next_device(&bus);
35 }
36
** CID 376207: Uninitialized variables (UNINIT)
/cmd/eficonfig.c: 2325 in eficonfig_delete_invalid_boot_option()
________________________________________________________________________________________________________
*** CID 376207: Uninitialized variables (UNINIT)
/cmd/eficonfig.c: 2325 in eficonfig_delete_invalid_boot_option()
2319 }
2320 next:
2321 free(load_option);
2322 }
2323
2324 out:
>>> CID 376207: Uninitialized variables (UNINIT)
>>> Using uninitialized value "ret".
2325 return ret;
2326 }
2327
2328 /**
2329 * eficonfig_generate_media_device_boot_option() - generate
the media device boot option
2330 *
** CID 376206: (CHECKED_RETURN)
/cmd/eficonfig.c: 127 in eficonfig_print_msg()
/cmd/eficonfig.c: 134 in eficonfig_print_msg()
________________________________________________________________________________________________________
*** CID 376206: (CHECKED_RETURN)
/cmd/eficonfig.c: 127 in eficonfig_print_msg()
121 * Return: status code
122 */
123 void eficonfig_print_msg(char *msg)
124 {
125 /* Flush input */
126 while (tstc())
>>> CID 376206: (CHECKED_RETURN)
>>> Calling "getchar()" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
127 getchar();
128
129 printf(ANSI_CURSOR_HIDE
130 ANSI_CLEAR_CONSOLE
131 ANSI_CURSOR_POSITION
132 "%s\n\n Press any key to continue", 3, 4, msg);
/cmd/eficonfig.c: 134 in eficonfig_print_msg()
128
129 printf(ANSI_CURSOR_HIDE
130 ANSI_CLEAR_CONSOLE
131 ANSI_CURSOR_POSITION
132 "%s\n\n Press any key to continue", 3, 4, msg);
133
>>> CID 376206: (CHECKED_RETURN)
>>> Calling "getchar()" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
134 getchar();
135 }
136
137 /**
138 * eficonfig_print_entry() - print each menu entry
139 *
** CID 376205: (TAINTED_SCALAR)
________________________________________________________________________________________________________
*** CID 376205: (TAINTED_SCALAR)
/test/test-main.c: 582 in ut_run_list()
576 printf("Running %d %s tests\n", count, category);
577
578 uts.of_root = gd_of_root();
579 uts.runs_per_test = runs_per_test;
580 if (fdt_action() == FDTCHK_COPY && gd->fdt_blob) {
581 uts.fdt_size = fdt_totalsize(gd->fdt_blob);
>>> CID 376205: (TAINTED_SCALAR)
>>> Passing tainted expression "uts.fdt_size" to "os_malloc", which uses it as an offset.
582 uts.fdt_copy = os_malloc(uts.fdt_size);
583 if (!uts.fdt_copy) {
584 printf("Out of memory for device tree copy\n");
585 return -ENOMEM;
586 }
587 memcpy(uts.fdt_copy, gd->fdt_blob, uts.fdt_size);
/test/test-main.c: 596 in ut_run_list()
590 ret = ut_run_tests(&uts, prefix, tests, count, select_name);
591
592 /* Best efforts only...ignore errors */
593 if (has_dm_tests)
594 dm_test_restore(uts.of_root);
595 if (IS_ENABLED(CONFIG_SANDBOX)) {
>>> CID 376205: (TAINTED_SCALAR)
>>> Passing tainted expression "*uts.fdt_copy" to "os_free", which uses it as an offset.
596 os_free(uts.fdt_copy);
597 os_free(uts.other_fdt);
598 }
599
600 if (uts.skip_count)
601 printf("Skipped: %d, ", uts.skip_count);
** CID 376204: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2854 in
efi_uninstall_multiple_protocol_interfaces_ext()
________________________________________________________________________________________________________
*** CID 376204: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2854 in
efi_uninstall_multiple_protocol_interfaces_ext()
2848 efi_uninstall_multiple_protocol_interfaces_ext(efi_handle_t
handle, ...)
2849 {
2850 EFI_ENTRY("%p", handle);
2851 efi_status_t ret;
2852 efi_va_list argptr;
2853
>>> CID 376204: Memory - illegal accesses (UNINIT)
>>> Using uninitialized value "argptr" when calling "__builtin_ms_va_start".
2854 efi_va_start(argptr, handle);
2855 ret =
efi_uninstall_multiple_protocol_interfaces_int(handle, argptr);
2856 efi_va_end(argptr);
2857 return EFI_EXIT(ret);
2858 }
2859
** CID 376203: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2764 in
efi_uninstall_multiple_protocol_interfaces_int()
________________________________________________________________________________________________________
*** CID 376203: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2764 in
efi_uninstall_multiple_protocol_interfaces_int()
2758 size_t i = 0;
2759 efi_va_list argptr_copy;
2760
2761 if (!handle)
2762 return EFI_INVALID_PARAMETER;
2763
>>> CID 376203: Memory - illegal accesses (UNINIT)
>>> Using uninitialized value "argptr_copy" when calling "__builtin_ms_va_copy".
2764 efi_va_copy(argptr_copy, argptr);
2765 for (;;) {
2766 protocol = efi_va_arg(argptr, efi_guid_t*);
2767 if (!protocol)
2768 break;
2769 protocol_interface = efi_va_arg(argptr, void*);
** CID 376202: Incorrect expression (IDENTICAL_BRANCHES)
/cmd/eficonfig.c: 1530 in eficonfig_edit_boot_option()
________________________________________________________________________________________________________
*** CID 376202: Incorrect expression (IDENTICAL_BRANCHES)
/cmd/eficonfig.c: 1530 in eficonfig_edit_boot_option()
1524 goto out;
1525 p = tmp;
1526 utf16_utf8_strncpy(&p, bo->optional_data,
u16_strlen(bo->optional_data));
1527 }
1528
1529 ret = eficonfig_set_boot_option(varname, final_dp,
final_dp_size, bo->description, tmp);
>>> CID 376202: Incorrect expression (IDENTICAL_BRANCHES)
>>> The same code is executed when the condition "ret != 0UL" is true or false, because the code in the if-then branch and after the if statement is identical. Should the if statement be removed?
1530 if (ret != EFI_SUCCESS)
1531 goto out;
1532 out:
1533 free(tmp);
1534 free(bo->optional_data);
1535 free(bo->description);
** CID 376201: Error handling issues (CHECKED_RETURN)
________________________________________________________________________________________________________
*** CID 376201: Error handling issues (CHECKED_RETURN)
/drivers/scsi/sandbox_scsi.c: 54 in sandbox_scsi_exec()
48 ret);
49 return ret;
50 } else if (ret == SCSI_EMUL_DO_READ && priv->fd != -1) {
51 long bytes_read;
52
53 log_debug("read %x %x\n", info->seek_block, info->read_len);
>>> CID 376201: Error handling issues (CHECKED_RETURN)
>>> Calling "os_lseek(priv->fd, info->seek_block * info->block_size, 0)" without checking return value. It wraps a library function that may fail and return an error code.
54 os_lseek(priv->fd, info->seek_block * info->block_size,
55 OS_SEEK_SET);
56 bytes_read = os_read(priv->fd, req->pdata, info->buff_used);
57 if (bytes_read < 0)
58 return bytes_read;
59 if (bytes_read != info->buff_used)
** CID 376200: API usage errors (VARARGS)
/lib/efi_loader/efi_boottime.c: 2787 in
efi_uninstall_multiple_protocol_interfaces_int()
________________________________________________________________________________________________________
*** CID 376200: API usage errors (VARARGS)
/lib/efi_loader/efi_boottime.c: 2787 in
efi_uninstall_multiple_protocol_interfaces_int()
2781 }
2782 goto out;
2783 }
2784
2785 /* If an error occurred undo all changes. */
2786 for (; i; --i) {
>>> CID 376200: API usage errors (VARARGS)
>>> Calling va_arg on va_list "argptr_copy", which has not been prepared with va_start().
2787 protocol = efi_va_arg(argptr_copy, efi_guid_t*);
2788 protocol_interface = efi_va_arg(argptr_copy, void*);
2789
EFI_CALL(efi_install_protocol_interface(&handle, protocol,
2790
EFI_NATIVE_INTERFACE,
2791
protocol_interface));
2792 }
** CID 376199: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2733 in
efi_install_multiple_protocol_interfaces_ext()
________________________________________________________________________________________________________
*** CID 376199: Memory - illegal accesses (UNINIT)
/lib/efi_loader/efi_boottime.c: 2733 in
efi_install_multiple_protocol_interfaces_ext()
2727 efi_install_multiple_protocol_interfaces_ext(efi_handle_t *handle, ...)
2728 {
2729 EFI_ENTRY("%p", handle);
2730 efi_status_t ret;
2731 efi_va_list argptr;
2732
>>> CID 376199: Memory - illegal accesses (UNINIT)
>>> Using uninitialized value "argptr" when calling "__builtin_ms_va_start".
2733 efi_va_start(argptr, handle);
2734 ret =
efi_install_multiple_protocol_interfaces_int(handle, argptr);
2735 efi_va_end(argptr);
2736 return EFI_EXIT(ret);
2737 }
2738
** CID 376198: Insecure data handling (TAINTED_SCALAR)
/boot/image-fit.c: 1917 in fit_conf_get_prop_node()
________________________________________________________________________________________________________
*** CID 376198: Insecure data handling (TAINTED_SCALAR)
/boot/image-fit.c: 1917 in fit_conf_get_prop_node()
1911
1912 count = fit_conf_get_prop_node_count(fit, noffset, prop_name);
1913 if (count < 0)
1914 return count;
1915
1916 /* check each image in the list */
>>> CID 376198: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "count" as a loop boundary.
1917 for (i = 0; i < count; i++) {
1918 enum image_phase_t phase;
1919 int ret, node;
1920
1921 node = fit_conf_get_prop_node_index(fit,
noffset, prop_name, i);
1922 ret = fit_image_get_phase(fit, node, &phase);
** CID 376197: Incorrect expression (UNUSED_VALUE)
/cmd/sf.c: 242 in spi_flash_update()
________________________________________________________________________________________________________
*** CID 376197: Incorrect expression (UNUSED_VALUE)
/cmd/sf.c: 242 in spi_flash_update()
236 scale = (end - buf) / 100;
237 cmp_buf = memalign(ARCH_DMA_MINALIGN, flash->sector_size);
238 if (cmp_buf) {
239 ulong last_update = get_timer(0);
240
241 for (; buf < end && !err_oper; buf += todo,
offset += todo) {
>>> CID 376197: Incorrect expression (UNUSED_VALUE)
>>> Assigning value from "({...; (__min1 < __min2) ? __min1 : __min2;})" to "todo" here, but that stored value is overwritten before it can be used.
242 todo = min_t(size_t, end - buf,
flash->sector_size);
243 todo = min_t(size_t, end - buf,
244 flash->sector_size -
(offset % flash->sector_size));
245 if (get_timer(last_update) > 100) {
246 printf(" \rUpdating, %zu%% %lu B/s",
247 100 - (end - buf) / scale,
** CID 376196: Integer handling issues (NEGATIVE_RETURNS)
________________________________________________________________________________________________________
*** CID 376196: Integer handling issues (NEGATIVE_RETURNS)
/boot/bootdev-uclass.c: 202 in bootdev_list()
196 printf("--- ------ ------ -------- ------------------\n");
197 if (probe)
198 ret = uclass_first_device_check(UCLASS_BOOTDEV, &dev);
199 else
200 ret = uclass_find_first_device(UCLASS_BOOTDEV, &dev);
201 for (i = 0; dev; i++) {
>>> CID 376196: Integer handling issues (NEGATIVE_RETURNS)
>>> "ret" is passed to a parameter that cannot be negative.
202 printf("%3x [ %c ] %6s %-9.9s %s\n", dev_seq(dev),
203 device_active(dev) ? '+' : ' ',
204 ret ? simple_itoa(ret) : "OK",
205
dev_get_uclass_name(dev_get_parent(dev)), dev->name);
206 if (probe)
207 ret = uclass_next_device_check(&dev);
** CID 376195: Uninitialized variables (UNINIT)
/lib/efi_loader/efi_boottime.c: 2776 in
efi_uninstall_multiple_protocol_interfaces_int()
________________________________________________________________________________________________________
*** CID 376195: Uninitialized variables (UNINIT)
/lib/efi_loader/efi_boottime.c: 2776 in
efi_uninstall_multiple_protocol_interfaces_int()
2770 ret = efi_uninstall_protocol(handle, protocol,
2771 protocol_interface);
2772 if (ret != EFI_SUCCESS)
2773 break;
2774 i++;
2775 }
>>> CID 376195: Uninitialized variables (UNINIT)
>>> Using uninitialized value "ret".
2776 if (ret == EFI_SUCCESS) {
2777 /* If the last protocol has been removed,
delete the handle. */
2778 if (list_empty(&handle->protocols)) {
2779 list_del(&handle->link);
2780 free(handle);
2781 }
** CID 376194: Null pointer dereferences (REVERSE_INULL)
/drivers/block/blk-uclass.c: 626 in blk_next_device_err()
________________________________________________________________________________________________________
*** CID 376194: Null pointer dereferences (REVERSE_INULL)
/drivers/block/blk-uclass.c: 626 in blk_next_device_err()
620 return -ENODEV;
621 }
622
623 int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp)
624 {
625 for (uclass_next_device(devp);
>>> CID 376194: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "*devp" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
626 *devp;
627 uclass_next_device(devp)) {
628 if (!blk_flags_check(*devp, flags))
629 return 0;
630 }
631
________________________________________________________________________________________________________
----- End forwarded message -----
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] <62df3a0cb9fd2_30ed5f2acd4da7b9a431758@prd-scan-dashboard-0.mail>
@ 2022-07-26 4:22 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2022-07-26 4:22 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List, Steffen Jaeckel
Hello Tom,
could you, please, have a look at the problems reported by Coverity
concerning code introduced by you into U-Boot.
For SHA256_Update_recycle() I guess you just have to change the
signature of the function to
SHA256_Update_recycled (SHA256_CTX *ctx,
unsigned char *block, size_t len)
Looking at
https://scan8.scan.coverity.com/reports.htm#v40863/p10710/fileInstanceId=59559157&defectInstanceId=12260012&mergedDefectId=355364
https://scan8.scan.coverity.com/reports.htm#v40863/p10710/fileInstanceId=59559157&defectInstanceId=12260012&mergedDefectId=355365
and
https://scan8.scan.coverity.com/reports.htm#v40863/p10710/fileInstanceId=59559157&defectInstanceId=12260012&mergedDefectId=355366
I think the issues are false positives:
Coverity ignores that if the sha256_update() is called will length < 64
sha256_process() will be called with blocks = 0 and will not access the
buffer.
Best regards
Heinrich
-------- Forwarded Message --------
Subject: New Defects reported by Coverity Scan for Das U-Boot
Date: Tue, 26 Jul 2022 00:49:17 +0000 (UTC)
From: scan-admin@coverity.com
To: xypron.glpk@gmx.de
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.
2 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 3 of 3 defect(s)
** CID 355366: (OVERRUN)
________________________________________________________________________________________________________
*** CID 355366: (OVERRUN)
/lib/crypt/crypt-sha256.c: 104 in SHA256_Update_recycled()
98 SHA256_Update_recycled (SHA256_CTX *ctx,
99 unsigned char block[32], size_t len)
100 {
101 size_t cnt;
102 for (cnt = len; cnt >= 32; cnt -= 32)
103 SHA256_Update (ctx, block, 32);
>>> CID 355366: (OVERRUN)
>>> Overrunning buffer pointed to by "(void const *)block" of 32 bytes by passing it to a function which accesses it at byte offset 63.
104 SHA256_Update (ctx, block, cnt);
105 }
106 107 void
108 crypt_sha256crypt_rn (const char *phrase, size_t phr_size,
109 const char *setting, size_t ARG_UNUSED
(set_size),
/lib/crypt/crypt-sha256.c: 103 in SHA256_Update_recycled()
97 static void
98 SHA256_Update_recycled (SHA256_CTX *ctx,
99 unsigned char block[32], size_t len)
100 {
101 size_t cnt;
102 for (cnt = len; cnt >= 32; cnt -= 32)
>>> CID 355366: (OVERRUN)
>>> Overrunning buffer pointed to by "(void const *)block" of 32 bytes by passing it to a function which accesses it at byte offset 63.
103 SHA256_Update (ctx, block, 32);
104 SHA256_Update (ctx, block, cnt);
105 }
106 107 void
108 crypt_sha256crypt_rn (const char *phrase, size_t phr_size,
** CID 355365: Memory - corruptions (OVERRUN)
________________________________________________________________________________________________________
*** CID 355365: Memory - corruptions (OVERRUN)
/lib/crypt/crypt-sha256.c: 212 in crypt_sha256crypt_rn()
206 characters and it ends at the first `$' character (for
207 compatibility with existing implementations). */
208 SHA256_Update (ctx, salt, salt_size);
209 210 /* Add for any character in the phrase one byte of the
alternate sum. */
211 for (cnt = phr_size; cnt > 32; cnt -= 32)
>>> CID 355365: Memory - corruptions (OVERRUN)
>>> Overrunning buffer pointed to by "(void const *)result" of 32 bytes by passing it to a function which accesses it at byte offset 63.
212 SHA256_Update (ctx, result, 32);
213 SHA256_Update (ctx, result, cnt);
214 215 /* Take the binary representation of the length of the
phrase and for every
216 1 add the alternate sum, for every 0 the phrase. */
217 for (cnt = phr_size; cnt > 0; cnt >>= 1)
** CID 355364: (OVERRUN)
________________________________________________________________________________________________________
*** CID 355364: (OVERRUN)
/lib/sha256.c: 259 in sha256_finish()
253 PUT_UINT32_BE(low, msglen, 4);
254 255 last = ctx->total[0] & 0x3F;
256 padn = (last < 56) ? (56 - last) : (120 - last);
257 258 sha256_update(ctx, sha256_padding, padn);
>>> CID 355364: (OVERRUN)
>>> Overrunning array "msglen" of 8 bytes by passing it to a function which accesses it at byte offset 63.
259 sha256_update(ctx, msglen, 8);
260 261 PUT_UINT32_BE(ctx->state[0], digest, 0);
262 PUT_UINT32_BE(ctx->state[1], digest, 4);
263 PUT_UINT32_BE(ctx->state[2], digest, 8);
264 PUT_UINT32_BE(ctx->state[3], digest, 12);
/lib/sha256.c: 259 in sha256_finish()
253 PUT_UINT32_BE(low, msglen, 4);
254 255 last = ctx->total[0] & 0x3F;
256 padn = (last < 56) ? (56 - last) : (120 - last);
257 258 sha256_update(ctx, sha256_padding, padn);
>>> CID 355364: (OVERRUN)
>>> Overrunning array "msglen" of 8 bytes by passing it to a function which accesses it at byte offset 63.
259 sha256_update(ctx, msglen, 8);
260 261 PUT_UINT32_BE(ctx->state[0], digest, 0);
262 PUT_UINT32_BE(ctx->state[1], digest, 4);
263 PUT_UINT32_BE(ctx->state[2], digest, 8);
264 PUT_UINT32_BE(ctx->state[3], digest, 12);
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3D2T0s_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2XoD3RFHsuIXMFMppPhcX3i-2BylqPVMQRSkcH-2F8FH0yrtiNsTyqrACwgwKzcFMo110d4rbYxVU-2B6HUewkm6-2BnWaHjEY6qmqSh3JibC9pdT8olo3BdbSy-2BWanWn1DBtOw1z1cdAbywwX9dt2U78a3fVdmOhb2POgsi0MvPp4Pxgp4Cg-3D-3D
To manage Coverity Scan email notifications for "xypron.glpk@gmx.de",
click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXx4Y-2F1WK-2FIlbEOzfoxXLI-2FdwA0wwGn90rGGrBgiHW-2ByLDLbUOEV7XOvtc9zJmj9LPyrT06WSaMnNrm6wfrUN-2BXuWoaHdqOoEyL7CQlGSiE-2BfE-3D_9qC_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2XoD3RFHsuIXMFMppPhcX3iF6KnEIxQAjMHO-2BlD-2FPGZz4TDSk0BBoeIgWfCDpuLTBt0y-2B4v9hleXOTCQWQXpAtOvLz9f5xcEFBHkc8v8-2FEgrl-2B-2FxBUaiZwIAadIw6kkwIOi1-2BjFknesS-2FQN5pLywQA-2FRiTVFu8P4KaYNq7QGyQkrQ-3D-3D
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] <6082f7faa423_5762a2b148d4af9a86820@prd-scan-dashboard-0.mail>
@ 2021-04-24 4:52 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2021-04-24 4:52 UTC (permalink / raw)
To: u-boot
On 4/23/21 6:38 PM, scan-admin at coverity.com wrote:
> 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 331185: Insecure data handling (TAINTED_SCALAR)
> /lib/lz4.c: 143 in LZ4_decompress_generic()
>
>
> ________________________________________________________________________________________________________
> *** CID 331185: Insecure data handling (TAINTED_SCALAR)
> /lib/lz4.c: 143 in LZ4_decompress_generic()
> 137 }
> 138 else
> 139 {
> 140 if ((!endOnInput) && (cpy != oend)) goto _output_error; /* Error : block decoding must stop exactly there */
> 141 if ((endOnInput) && ((ip+length != iend) || (cpy > oend))) goto _output_error; /* Error : input must be consumed */
> 142 }
>>>> CID 331185: Insecure data handling (TAINTED_SCALAR)
>>>> Passing tainted variable "length" to a tainted sink. [Note: The source code implementation of the function has been overridden by a builtin model.]
> 143 memcpy(op, ip, length);
> 144 ip += length;
> 145 op += length;
> 146 break; /* Necessarily EOF, due to parsing restrictions */
> 147 }
> 148 LZ4_wildCopy(op, ip, cpy);
>
> ** CID 331184: Memory - corruptions (OVERRUN)
> /cmd/stackprot_test.c: 14 in do_test_stackprot_fail()
>
>
> ________________________________________________________________________________________________________
> *** CID 331184: Memory - corruptions (OVERRUN)
> /cmd/stackprot_test.c: 14 in do_test_stackprot_fail()
> 8
> 9 static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> 10 char *const argv[])
> 11 {
Hello Tom,
please, mark this finding as intentional in Coverity.
> 12 char a[128];
> 13
>>>> CID 331184: Memory - corruptions (OVERRUN)
>>>> Overrunning array "a" of 128 bytes by passing it to a function which accesses it at byte offset 511 using argument "512UL". [Note: The source code implementation of the function has been overridden by a builtin model.]
> 14 memset(a, 0xa5, 512);
> 15 return 0;
> 16 }
> 17
> 18 U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
>
> ** CID 331183: Memory - corruptions (BUFFER_SIZE)
> /cmd/stackprot_test.c: 14 in do_test_stackprot_fail()
>
>
> ________________________________________________________________________________________________________
> *** CID 331183: Memory - corruptions (BUFFER_SIZE)
> /cmd/stackprot_test.c: 14 in do_test_stackprot_fail()
same here
Best regards
Heinrich
> 8
> 9 static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> 10 char *const argv[])
> 11 {
> 12 char a[128];
> 13
>>>> CID 331183: Memory - corruptions (BUFFER_SIZE)
>>>> You might overrun the 128 byte destination string "a" by writing the maximum 512 bytes from "165".
> 14 memset(a, 0xa5, 512);
> 15 return 0;
> 16 }
> 17
> 18 U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3DVLO3_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2X9PIw0aqIVMZlR6cmf9w8prU0ddkFkhQg-2B6p8UvlY-2FM51TBl-2FigNKw0KCrquAEkBb2jC3ZnWBwbVEZhLkDdq-2FMFkIpcluF4NvkPbaQ8l7PMYWmxLPqhtFLo01zbJ6O05zRrW9MzeWZiF82fugYqxJUGlLrQGmeTLuFDr2CDzEGJg-3D-3D
>
> To manage Coverity Scan email notifications for "xypron.glpk at gmx.de", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXx4Y-2F1WK-2FIlbEOzfoxXLI-2FdwA0wwGn90rGGrBgiHW-2ByLDLbUOEV7XOvtc9zJmj9LPyrT06WSaMnNrm6wfrUN-2BXuWoaHdqOoEyL7CQlGSiE-2BfE-3DtDQo_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2X9PIw0aqIVMZlR6cmf9w8pA8-2FW82eD6YTWlxlNXjrDSc-2B-2BfgU0QJMdYPvNOg-2Brk8a8VMShB-2FvhmE5GTrUF2ImOx4sRousy5Sh2qX6apgHec8wEC6ZWvhuro1Ua3CVllqnKzeW-2FmUepM3XfZqtYssGH0ujkCtgvKvxZfYpXxJdKdg-3D-3D
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
2020-05-26 20:36 ` Heinrich Schuchardt
@ 2020-05-26 20:48 ` Tom Rini
0 siblings, 0 replies; 36+ messages in thread
From: Tom Rini @ 2020-05-26 20:48 UTC (permalink / raw)
To: u-boot
On Tue, May 26, 2020 at 10:36:44PM +0200, Heinrich Schuchardt wrote:
> On 26.05.20 22:10, Tom Rini wrote:
> > On Tue, May 26, 2020 at 10:02:36PM +0200, Heinrich Schuchardt
> > wrote:
> >> On 26.05.20 20:40, Tom Rini wrote:
> >>> Ah, I thought you might not have been part of Coverity.
> >>> https://scan.coverity.com/projects/das-u-boot is where to
> >>> start, it will take GitHub credentials and then I can approve
> >>> you.
> >>
> >> Thanks for granting access. In the GUI one can really drill down
> >> into the explanation of the problem. This is very helpful.
> >
> > And thanks for digging more!
> >
> >>
> >>> ** CID 303760: (TAINTED_SCALAR)
> >>>
> >>>
> >>>
> >> ________________________________________________________________________________________________________
> >>>
> >>
> *** CID 303760: (TAINTED_SCALAR)
> >>> /cmd/efidebug.c: 938 in show_efi_boot_order() 932
> >>> } 933 p = label; 934
> >>> utf16_utf8_strncpy(&p, lo.label, label_len16); 935
> >>> printf("%2d: %s: %s\n", i + 1, var_name, label); 936
> >>> free(label); 937
> >>>>>> CID 303760: (TAINTED_SCALAR) Passing tainted variable
> >>>>>> "data" to a tainted sink.
> >>> 938 free(data); 939 } 940
> >>> out: 941 free(bootorder); 942 943
> >>> return ret; /cmd/efidebug.c: 929 in show_efi_boot_order() 923
> >>> efi_deserialize_load_option(&lo, data); 924 925
> >>> label_len16 = u16_strlen(lo.label); 926
> >>> label_len = utf16_utf8_strnlen(lo.label,
> >> label_len16);
> >>> 927 label = malloc(label_len + 1); 928
> >>> if (!label) {
> >>>>>> CID 303760: (TAINTED_SCALAR) Passing tainted variable
> >>>>>> "data" to a tainted sink.
> >>> 929 free(data); 930
> >>> ret = CMD_RET_FAILURE; 931 goto
> >>> out; 932 } 933 p =
> >>> label; 934 utf16_utf8_strncpy(&p, lo.label,
> >>> label_len16);
> >>
> >> In CID 303760 the logic is as follows:
> >>
> >> In show_efi_boot_order() we malloc() memory. The allocated buffer
> >> is filled via byte swapping (get_unaligned_le16(),
> >> get_unaligned_le32()).
> >>
> >> Here comes Coverity's logic: "byte_swapping: Performing a byte
> >> swapping operation on ... implies that it came from an external
> >> source, and is therefore tainted."
> >>
> >> Now we pass the pointer to free(). Free() looks at 16 bytes
> >> preceding the pointer. Therefore free() is considered a tainted
> >> sink and an issue is raised.
> >>
> >> https://community.synopsys.com/s/article/From-Case-Clearing-TAINTED-STRING
> >>
> >>
> suggests to use Coverity specific comments to mark cleansing functions.
> >> This is not what I am inclined to do.
> >>
> >> CCing Takahiro as he had a hand in this code.
> >
> > So, option B on that link is what I was thinking about which is
> > creating a function in the model file to tell Coverity it's
> > handled. I was going to include what ours was already as I thought
> > I had written one, but there's not one in the dashboard currently.
> > And frankly a drawback of Coverity is that you can't iterate on
> > testing those kind of things easily.
>
> Here are example model files for Coverity:
>
> https://github.com/qemu/qemu/blob/master/scripts/coverity-model.c
> https://github.com/python/cpython/blob/master/Misc/coverity_model.c
>
> How many functions do you think we will have to maintain in the model
> file? Who will take the effort?
Ah yes, I think I looked at those a while ago and didn't come up with
anything that reduced our defects so I set it aside to look harder at
later. And haven't yet cycled back.
I would say once we have an initial functional skeleton in, any time
someone sees a Coverity defect that's a false positive and wants to
write a model function to cover it rather than just close it out in the
dashboard, we'll get an update to it and I'll push it to Coverity.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200526/79606bb2/attachment.sig>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
2020-05-26 20:10 ` Tom Rini
@ 2020-05-26 20:36 ` Heinrich Schuchardt
2020-05-26 20:48 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2020-05-26 20:36 UTC (permalink / raw)
To: u-boot
On 26.05.20 22:10, Tom Rini wrote:
> On Tue, May 26, 2020 at 10:02:36PM +0200, Heinrich Schuchardt
> wrote:
>> On 26.05.20 20:40, Tom Rini wrote:
>>> Ah, I thought you might not have been part of Coverity.
>>> https://scan.coverity.com/projects/das-u-boot is where to
>>> start, it will take GitHub credentials and then I can approve
>>> you.
>>
>> Thanks for granting access. In the GUI one can really drill down
>> into the explanation of the problem. This is very helpful.
>
> And thanks for digging more!
>
>>
>>> ** CID 303760: (TAINTED_SCALAR)
>>>
>>>
>>>
>> ________________________________________________________________________________________________________
>>>
>>
*** CID 303760: (TAINTED_SCALAR)
>>> /cmd/efidebug.c: 938 in show_efi_boot_order() 932
>>> } 933 p = label; 934
>>> utf16_utf8_strncpy(&p, lo.label, label_len16); 935
>>> printf("%2d: %s: %s\n", i + 1, var_name, label); 936
>>> free(label); 937
>>>>>> CID 303760: (TAINTED_SCALAR) Passing tainted variable
>>>>>> "data" to a tainted sink.
>>> 938 free(data); 939 } 940
>>> out: 941 free(bootorder); 942 943
>>> return ret; /cmd/efidebug.c: 929 in show_efi_boot_order() 923
>>> efi_deserialize_load_option(&lo, data); 924 925
>>> label_len16 = u16_strlen(lo.label); 926
>>> label_len = utf16_utf8_strnlen(lo.label,
>> label_len16);
>>> 927 label = malloc(label_len + 1); 928
>>> if (!label) {
>>>>>> CID 303760: (TAINTED_SCALAR) Passing tainted variable
>>>>>> "data" to a tainted sink.
>>> 929 free(data); 930
>>> ret = CMD_RET_FAILURE; 931 goto
>>> out; 932 } 933 p =
>>> label; 934 utf16_utf8_strncpy(&p, lo.label,
>>> label_len16);
>>
>> In CID 303760 the logic is as follows:
>>
>> In show_efi_boot_order() we malloc() memory. The allocated buffer
>> is filled via byte swapping (get_unaligned_le16(),
>> get_unaligned_le32()).
>>
>> Here comes Coverity's logic: "byte_swapping: Performing a byte
>> swapping operation on ... implies that it came from an external
>> source, and is therefore tainted."
>>
>> Now we pass the pointer to free(). Free() looks at 16 bytes
>> preceding the pointer. Therefore free() is considered a tainted
>> sink and an issue is raised.
>>
>> https://community.synopsys.com/s/article/From-Case-Clearing-TAINTED-STRING
>>
>>
suggests to use Coverity specific comments to mark cleansing functions.
>> This is not what I am inclined to do.
>>
>> CCing Takahiro as he had a hand in this code.
>
> So, option B on that link is what I was thinking about which is
> creating a function in the model file to tell Coverity it's
> handled. I was going to include what ours was already as I thought
> I had written one, but there's not one in the dashboard currently.
> And frankly a drawback of Coverity is that you can't iterate on
> testing those kind of things easily.
Here are example model files for Coverity:
https://github.com/qemu/qemu/blob/master/scripts/coverity-model.c
https://github.com/python/cpython/blob/master/Misc/coverity_model.c
How many functions do you think we will have to maintain in the model
file? Who will take the effort?
Best regards
Heinrich
>
> Option C is to just mark this (and the similar ones you can see via
> the dashboard) as false positive.
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
2020-05-26 20:02 ` Heinrich Schuchardt
@ 2020-05-26 20:10 ` Tom Rini
2020-05-26 20:36 ` Heinrich Schuchardt
0 siblings, 1 reply; 36+ messages in thread
From: Tom Rini @ 2020-05-26 20:10 UTC (permalink / raw)
To: u-boot
On Tue, May 26, 2020 at 10:02:36PM +0200, Heinrich Schuchardt wrote:
> On 26.05.20 20:40, Tom Rini wrote:
> > Ah, I thought you might not have been part of Coverity.
> > https://scan.coverity.com/projects/das-u-boot is where to start, it will
> > take GitHub credentials and then I can approve you.
>
> Thanks for granting access. In the GUI one can really drill down into
> the explanation of the problem. This is very helpful.
And thanks for digging more!
>
> > ** CID 303760: (TAINTED_SCALAR)
> >
> >
> >
> ________________________________________________________________________________________________________
> > *** CID 303760: (TAINTED_SCALAR)
> > /cmd/efidebug.c: 938 in show_efi_boot_order()
> > 932 }
> > 933 p = label;
> > 934 utf16_utf8_strncpy(&p, lo.label, label_len16);
> > 935 printf("%2d: %s: %s\n", i + 1, var_name, label);
> > 936 free(label);
> > 937
> >>>> CID 303760: (TAINTED_SCALAR)
> >>>> Passing tainted variable "data" to a tainted sink.
> > 938 free(data);
> > 939 }
> > 940 out:
> > 941 free(bootorder);
> > 942
> > 943 return ret;
> > /cmd/efidebug.c: 929 in show_efi_boot_order()
> > 923 efi_deserialize_load_option(&lo, data);
> > 924
> > 925 label_len16 = u16_strlen(lo.label);
> > 926 label_len = utf16_utf8_strnlen(lo.label,
> label_len16);
> > 927 label = malloc(label_len + 1);
> > 928 if (!label) {
> >>>> CID 303760: (TAINTED_SCALAR)
> >>>> Passing tainted variable "data" to a tainted sink.
> > 929 free(data);
> > 930 ret = CMD_RET_FAILURE;
> > 931 goto out;
> > 932 }
> > 933 p = label;
> > 934 utf16_utf8_strncpy(&p, lo.label, label_len16);
>
> In CID 303760 the logic is as follows:
>
> In show_efi_boot_order() we malloc() memory. The allocated buffer is
> filled via byte swapping (get_unaligned_le16(), get_unaligned_le32()).
>
> Here comes Coverity's logic: "byte_swapping: Performing a byte swapping
> operation on ... implies that it came from an external source, and is
> therefore tainted."
>
> Now we pass the pointer to free(). Free() looks at 16 bytes preceding
> the pointer. Therefore free() is considered a tainted sink and an issue
> is raised.
>
> https://community.synopsys.com/s/article/From-Case-Clearing-TAINTED-STRING
> suggests to use Coverity specific comments to mark cleansing functions.
> This is not what I am inclined to do.
>
> CCing Takahiro as he had a hand in this code.
So, option B on that link is what I was thinking about which is creating
a function in the model file to tell Coverity it's handled. I was
going to include what ours was already as I thought I had written one,
but there's not one in the dashboard currently. And frankly a drawback
of Coverity is that you can't iterate on testing those kind of things
easily.
Option C is to just mark this (and the similar ones you can see via the
dashboard) as false positive.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200526/d26215cb/attachment.sig>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Fwd: New Defects reported by Coverity Scan for Das U-Boot
[not found] ` <20200526184027.GJ12717@bill-the-cat>
@ 2020-05-26 20:02 ` Heinrich Schuchardt
2020-05-26 20:10 ` Tom Rini
0 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2020-05-26 20:02 UTC (permalink / raw)
To: u-boot
On 26.05.20 20:40, Tom Rini wrote:
> Ah, I thought you might not have been part of Coverity.
> https://scan.coverity.com/projects/das-u-boot is where to start, it will
> take GitHub credentials and then I can approve you.
Thanks for granting access. In the GUI one can really drill down into
the explanation of the problem. This is very helpful.
> ** CID 303760: (TAINTED_SCALAR)
>
>
>
________________________________________________________________________________________________________
> *** CID 303760: (TAINTED_SCALAR)
> /cmd/efidebug.c: 938 in show_efi_boot_order()
> 932 }
> 933 p = label;
> 934 utf16_utf8_strncpy(&p, lo.label, label_len16);
> 935 printf("%2d: %s: %s\n", i + 1, var_name, label);
> 936 free(label);
> 937
>>>> CID 303760: (TAINTED_SCALAR)
>>>> Passing tainted variable "data" to a tainted sink.
> 938 free(data);
> 939 }
> 940 out:
> 941 free(bootorder);
> 942
> 943 return ret;
> /cmd/efidebug.c: 929 in show_efi_boot_order()
> 923 efi_deserialize_load_option(&lo, data);
> 924
> 925 label_len16 = u16_strlen(lo.label);
> 926 label_len = utf16_utf8_strnlen(lo.label,
label_len16);
> 927 label = malloc(label_len + 1);
> 928 if (!label) {
>>>> CID 303760: (TAINTED_SCALAR)
>>>> Passing tainted variable "data" to a tainted sink.
> 929 free(data);
> 930 ret = CMD_RET_FAILURE;
> 931 goto out;
> 932 }
> 933 p = label;
> 934 utf16_utf8_strncpy(&p, lo.label, label_len16);
In CID 303760 the logic is as follows:
In show_efi_boot_order() we malloc() memory. The allocated buffer is
filled via byte swapping (get_unaligned_le16(), get_unaligned_le32()).
Here comes Coverity's logic: "byte_swapping: Performing a byte swapping
operation on ... implies that it came from an external source, and is
therefore tainted."
Now we pass the pointer to free(). Free() looks at 16 bytes preceding
the pointer. Therefore free() is considered a tainted sink and an issue
is raised.
https://community.synopsys.com/s/article/From-Case-Clearing-TAINTED-STRING
suggests to use Coverity specific comments to mark cleansing functions.
This is not what I am inclined to do.
CCing Takahiro as he had a hand in this code.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2024-04-22 21:48 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <611aaf735d268_21438d2b07184e399c79439@prd-scan-dashboard-0.mail>
2021-08-17 5:21 ` Fwd: New Defects reported by Coverity Scan for Das U-Boot Heinrich Schuchardt
2021-08-17 15:17 ` Tom Rini
2024-04-22 21:48 Tom Rini
-- strict thread matches above, loose matches on Subject: below --
2024-01-29 23:55 Tom Rini
2024-01-30 8:14 ` Heinrich Schuchardt
[not found] <20240127154018.GC785631@bill-the-cat>
2024-01-27 20:56 ` Heinrich Schuchardt
2024-01-28 8:51 ` Heinrich Schuchardt
2024-01-22 23:52 Tom Rini
2024-01-22 23:30 Tom Rini
2024-01-23 8:15 ` Hugo Cornelis
[not found] <65a933ab652b3_da12cbd3e77f998728e5@prd-scan-dashboard-0.mail>
2024-01-19 8:47 ` Heinrich Schuchardt
2024-01-18 14:35 Tom Rini
2024-01-08 17:45 Tom Rini
2024-01-09 5:26 ` Sean Anderson
2024-01-09 22:18 ` Tom Rini
2023-08-21 21:09 Tom Rini
2023-08-24 9:27 ` Abdellatif El Khlifi
2023-08-28 16:09 ` Alvaro Fernando García
2023-08-28 16:11 ` Tom Rini
2023-10-20 11:57 ` Abdellatif El Khlifi
2023-10-25 14:57 ` Tom Rini
2023-10-25 15:12 ` Abdellatif El Khlifi
2023-10-25 15:15 ` Tom Rini
2023-10-31 14:21 ` Abdellatif El Khlifi
2023-05-08 20:20 Tom Rini
2023-05-15 21:59 ` Ehsan Mohandesi
2023-05-18 21:04 ` Sean Edmond
2023-02-14 14:26 Tom Rini
2022-11-21 19:43 Tom Rini
2022-11-09 15:40 Tom Rini
[not found] <62df3a0cb9fd2_30ed5f2acd4da7b9a431758@prd-scan-dashboard-0.mail>
2022-07-26 4:22 ` Heinrich Schuchardt
[not found] <6082f7faa423_5762a2b148d4af9a86820@prd-scan-dashboard-0.mail>
2021-04-24 4:52 ` Heinrich Schuchardt
[not found] <5ecd3c8249d1_d6f562acb748daf5820386@appnode-2.mail>
[not found] ` <CA+M6bX=AmT+SyM0Snt2POLy0-vpD__6CD4j6ifqMqh63yYJBLA@mail.gmail.com>
[not found] ` <8ea1ca2f-2826-58f2-4b6b-ed5cfe977467@gmx.de>
[not found] ` <20200526184027.GJ12717@bill-the-cat>
2020-05-26 20:02 ` Heinrich Schuchardt
2020-05-26 20:10 ` Tom Rini
2020-05-26 20:36 ` Heinrich Schuchardt
2020-05-26 20:48 ` Tom Rini
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.