From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 9 Oct 2018 15:55:59 +0900 Subject: [U-Boot] [PATCH 1/1] efi_loader: set image_base and image_size to correct values In-Reply-To: References: <20180930052601.7173-1-xypron.glpk@gmx.de> Message-ID: <20181009065558.GP32578@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sat, Oct 06, 2018 at 09:51:01AM +0200, Heinrich Schuchardt wrote: > On 09/30/2018 07:26 AM, Heinrich Schuchardt wrote: > > From: AKASHI Takahiro > > > > 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 > > > > This patch also reverts a patch, "efi_loader: save image relocation address > > and size" since newly added fields are no longer needed. > > > > Signed-off-by: AKASHI Takahiro > > > > Rebase the patch. Keep the relocation address in struct efi_image_object. > > We will use the address to free the image in UnloadImage. > > > > Signed-off-by: Heinrich Schuchardt > > --- > > lib/efi_loader/efi_image_loader.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > > index a18ce0a5705..39902152f3c 100644 > > --- a/lib/efi_loader/efi_image_loader.c > > +++ b/lib/efi_loader/efi_image_loader.c > > @@ -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); > > @@ -315,10 +312,10 @@ 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; > > } > > > > With this patch GRUB is not able to load the modules which are included > in grubaa64.efi Oh, really? > ## Starting EFI application at 40400000 ... > error: unknown filesystem. > Entering rescue mode... > grub rescue> > > Function grub_efi_modules_addr() expects image_base to point to the > unrelocated image: > > header = image->image_base; Here "image" points to 'grub' itself, right? If so, > coff_header = &(header->coff_header); > sections > = (struct grub_pe32_section_table *) ((char *) coff_header > + sizeof (*coff_header) > + > coff_header->optional_header_size); It seems to me that the above code shows that we should also copy PE headers, along with sections, after a new location is allocated in efi_load_pe(). Then my patch should work again, I didn't test it though. Thanks, -Takahiro Akashi > The UEFI SCT II specifcation test 5.3.1.1.11 requires > > Check on Application Images which have Unload function. > Unload field should be valid and its entry address should be within > the range of [ImageBase, ImageBase+ImageSize] > > @Leif > Any idea how to sort this out? > > Best regards > > Heinrich > > >