+ Stefan and Marek
On Monday 16 August 2021 15:57:26 Tom Rini wrote:
Hey all,
Can people please take a look? I can mark as intentional anything that really is intentional, thanks.
Hello Tom!
These kwbimage issues look to be a real issues. But I do not think that anybody touched these parts of kwbimage code recently. So looks like that Coverity must have run some more tests this time...
** 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,
There is really missing check that fopen() succeeded.
** 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),
There is code:
int csk_idx = image_get_csk_index(); ... if (csk_idx >= 16) { ... return 1; } ... &secure_hdr->csk[csk_idx] ...
And ->csk is defined as:
struct secure_hdr_v1 { .. struct pubkey_der_v1 csk[16] .. };
image_get_csk_index() returns int and it may returns also negative value on error. So there is really possible illegal memory access.
** 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
And there is also missing check that fopen() succeeded.