Hello Takahiro,
this relates to your patch
4540dabdcaca
efi_loader: image_loader: support image authentication
On 4/29/20 4:16 PM, Tom Rini wrote:
> Can you please look in to these, thanks!
>
> ---------- Forwarded message ---------
> From: <scan-ad...@coverity.com>
> Date: Tue, Apr 28, 2020 at 4:12 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.r...@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 12 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 12 of 12 defect(s)
>
>
> ** CID 300339: (ARRAY_VS_SINGLETON)
> /lib/efi_loader/efi_image_loader.c: 299 in efi_image_parse()
> /lib/efi_loader/efi_image_loader.c: 294 in efi_image_parse()
> /lib/efi_loader/efi_image_loader.c: 315 in efi_image_parse()
>
>
> ________________________________________________________________________________________________________
> *** CID 300339: (ARRAY_VS_SINGLETON)
> /lib/efi_loader/efi_image_loader.c: 299 in efi_image_parse()
> 293 if (nt64->OptionalHeader.NumberOfRvaAndSizes <=
> ctidx) {
> 294 efi_image_region_add(regs,
> 295 &opt->CheckSum + 1,
+1 is not adding the size of CheckSum to the address but the size of
IMAGE_OPTIONAL_HEADER64. This cannot be correct.
> 296 efi +
> opt->SizeOfHeaders, 0);
> 297 } else {
> 298 /* Skip Certificates Table */
>>>> CID 300339: (ARRAY_VS_SINGLETON)
>>>> Using "&opt->CheckSum" as an array. This might corrupt or
>>>> misinterpret adjacent memory locations.
> 299 efi_image_region_add(regs,
> 300 &opt->CheckSum + 1,
Same here
> 301
> &opt->DataDirectory[ctidx], 0);
> 302 efi_image_region_add(regs,
> 303
> &opt->DataDirectory[ctidx] + 1,
Same here
> 304 efi +
> opt->SizeOfHeaders, 0);
> /lib/efi_loader/efi_image_loader.c: 294 in efi_image_parse()
> 288 IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
> 289 IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
> 290
> 291 /* Skip CheckSum */
> 292 efi_image_region_add(regs, efi, &opt->CheckSum, 0);
> 293 if (nt64->OptionalHeader.NumberOfRvaAndSizes <=
> ctidx) {
>>>> CID 300339: (ARRAY_VS_SINGLETON)
>>>> Using "&opt->CheckSum" as an array. This might corrupt or
>>>> misinterpret adjacent memory locations.
> 294 efi_image_region_add(regs,
> 295 &opt->CheckSum + 1,
> 296 efi +
> opt->SizeOfHeaders, 0);
> 297 } else {
> 298 /* Skip Certificates Table */
> 299 efi_image_region_add(regs,
> /lib/efi_loader/efi_image_loader.c: 315 in efi_image_parse()
> 309 authoff = opt->DataDirectory[ctidx].VirtualAddress;
> 310 authsz = opt->DataDirectory[ctidx].Size;
> 311 } else if (nt->OptionalHeader.Magic ==
> IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> 312 IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
> 313
> 314 efi_image_region_add(regs, efi, &opt->CheckSum, 0);
>>>> CID 300339: (ARRAY_VS_SINGLETON)
>>>> Using "&opt->CheckSum" as an array. This might corrupt or
>>>> misinterpret adjacent memory locations.
> 315 efi_image_region_add(regs, &opt->CheckSum + 1,
Same here
> 316 &opt->DataDirectory[ctidx], 0);
> 317 efi_image_region_add(regs,
> &opt->DataDirectory[ctidx] + 1,
> 318 efi + opt->SizeOfHeaders, 0);
> 319
> 320 bytes_hashed = opt->SizeOfHeaders;
>
Please, check the coding and provide a patch.
Best regards
Heinrich
