On 10/11/18 1:11 PM, AKASHI Takahiro wrote: > Currently, image's image_base points to an address where the image was > temporarily uploaded for further loading. Since efi_loader relocates > the image to final destination, image_base and image_size should reflect > that. > > This bug was detected in UEFI SCT, "Loaded Image Protocol Test - test 2," > which shows that 'Unload' function doesn't fit into a range suggested by > image_base and image_size. > TestCase/UEFI/EFI/Protocol/LoadedImage/BlackBoxTest/ > LoadedImageBBTestMain.c:1002 > > Changes in this patch also includes: > * reverts a patch, "efi_loader: save image relocation address > and size" since newly added fields are no longer needed. > * copy PE headers as well since those information will be needed > for module loading, in particular, at gurb. > (This bug was reported by Heinrich.) > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > Reported-by: Heinrich Schuchardt <xypron.g...@gmx.de> > --- > lib/efi_loader/efi_image_loader.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/lib/efi_loader/efi_image_loader.c > b/lib/efi_loader/efi_image_loader.c > index a18ce0a5705e..d1b6c0d3cdf2 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -59,10 +59,10 @@ static efi_status_t efi_print_image_info(struct > efi_loaded_image_obj *obj, > { > printf("UEFI image"); > printf(" [0x%p:0x%p]", > - obj->reloc_base, obj->reloc_base + obj->reloc_size - 1); > - if (pc && pc >= obj->reloc_base && > - pc < obj->reloc_base + obj->reloc_size) > - printf(" pc=0x%zx", pc - obj->reloc_base); > + image->image_base, image->image_base + image->image_size - 1); > + if (pc && pc >= image->image_base && > + pc < image->image_base + image->image_size) > + printf(" pc=0x%zx", pc - image->image_base); > if (image->file_path) > printf(" '%pD'", image->file_path); > printf("\n"); > @@ -212,7 +212,6 @@ void *efi_load_pe(struct efi_loaded_image_obj *handle, > void *efi, > int rel_idx = IMAGE_DIRECTORY_ENTRY_BASERELOC; > void *entry; > uint64_t image_base; > - uint64_t image_size; > unsigned long virt_size = 0; > int supported = 0; > > @@ -256,7 +255,6 @@ void *efi_load_pe(struct efi_loaded_image_obj *handle, > void *efi, > IMAGE_NT_HEADERS64 *nt64 = (void *)nt; > IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader; > image_base = opt->ImageBase; > - image_size = opt->SizeOfImage; > efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); > efi_reloc = efi_alloc(virt_size, > loaded_image_info->image_code_type); > @@ -272,7 +270,6 @@ void *efi_load_pe(struct efi_loaded_image_obj *handle, > void *efi, > } else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) { > IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader; > image_base = opt->ImageBase; > - image_size = opt->SizeOfImage; > efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); > efi_reloc = efi_alloc(virt_size, > loaded_image_info->image_code_type); > @@ -291,6 +288,11 @@ void *efi_load_pe(struct efi_loaded_image_obj *handle, > void *efi, > return NULL; > } > > + /* Copy PE headers */ > + memcpy(efi_reloc, efi, sizeof(*dos) + sizeof(*nt) > + + nt->FileHeader.SizeOfOptionalHeader > + + num_sections * sizeof(IMAGE_SECTION_HEADER)); > +
Why is it needed to copy the header? Did you experience any error related to this header not copied? Best regards Heinrich > /* Load sections into RAM */ > for (i = num_sections - 1; i >= 0; i--) { > IMAGE_SECTION_HEADER *sec = §ions[i]; > @@ -315,10 +317,8 @@ void *efi_load_pe(struct efi_loaded_image_obj *handle, > void *efi, > invalidate_icache_all(); > > /* Populate the loaded image interface bits */ > - loaded_image_info->image_base = efi; > - loaded_image_info->image_size = image_size; > - handle->reloc_base = efi_reloc; > - handle->reloc_size = virt_size; > + loaded_image_info->image_base = efi_reloc; > + loaded_image_info->image_size = virt_size; > > return entry; > } > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot