On Mon, Aug 16, 2021 at 10:15:49PM +0200, Pali Rohár wrote: > + 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...
Yeah, that happens from time to 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. Since you've been in here and analyzed things (thanks!) can you make a few patches for things? -- Tom
signature.asc
Description: PGP signature