Re: [kvm-unit-tests PATCH v2 4/6] x86: efi_main: Self-relocate ELF .dynamic addresses

Krish Sadhukhan
2021-08-24T22:11:10Z

On 8/19/21 4:33 AM, Varad Gautam wrote:
> EFI expects a relocatable PE, and the loader will patch in the
> relocations from the COFF.
>
> Since we are wrapping an ELF into a PE here, the EFI loader will
> not handle ELF relocations, and we need to patch the ELF .dynamic
> section manually on early boot.
>
> Signed-off-by: Varad Gautam<varad.gautam@suse.com>
> ---
> x86/efi_main.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/x86/efi_main.c b/x86/efi_main.c
> index 237d4e7..be3f9ab 100644
> --- a/x86/efi_main.c
> +++ b/x86/efi_main.c
> @@ -1,9 +1,13 @@
> #include <alloc_phys.h>
> #include <linux/uefi.h>
> +#include <elf.h>
>
> unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
> efi_system_table_t *efi_system_table = NULL;
>
> +extern char ImageBase;
> +extern char _DYNAMIC;
> +
> static void efi_free_pool(void *ptr)
> {
> efi_bs_call(free_pool, ptr);
> @@ -93,11 +97,70 @@ static efi_status_t exit_efi(void *handle)
> return EFI_SUCCESS;
> }
>
> +static efi_status_t elf_reloc(unsigned long image_base, unsigned long dynamic)

Since this function is only relocating the dynamic section, we should
probably name it something like elf_reloc_dyn().

> +{
> + long relsz = 0, relent = 0;
> + Elf64_Rel *rel = 0;
> + Elf64_Dyn *dyn = (Elf64_Dyn *) dynamic;
> + unsigned long *addr;
> + int i;
> +
> + for (i = 0; dyn[i].d_tag != DT_NULL; i++) {
> + switch (dyn[i].d_tag) {
> + case DT_RELA:
> + rel = (Elf64_Rel *)
> + ((unsigned long) dyn[i].d_un.d_ptr + image_base);
> + break;
> + case DT_RELASZ:
> + relsz = dyn[i].d_un.d_val;
> + break;
> + case DT_RELAENT:
> + relent = dyn[i].d_un.d_val;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (!rel && relent == 0)
> + return EFI_SUCCESS;
> +
> + if (!rel || relent == 0)
> + return EFI_LOAD_ERROR;
> +
> + while (relsz > 0) {
> + /* apply the relocs */
> + switch (ELF64_R_TYPE (rel->r_info)) {
> + case R_X86_64_NONE:
> + break;
> + case R_X86_64_RELATIVE:
> + addr = (unsigned long *) (image_base + rel->r_offset);
> + *addr += image_base;
> + break;
> + default:
> + break;
> + }
> + rel = (Elf64_Rel *) ((char *) rel + relent);
> + relsz -= relent;
> + }
> + return EFI_SUCCESS;
> +}
> +
> unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> + unsigned long image_base, dyn;
> efi_system_table = sys_tab;
>
> exit_efi(handle);
>
> + image_base = (unsigned long) &ImageBase;
> + dyn = image_base + (unsigned long) &_DYNAMIC;
> +
> + /* The EFI loader does not handle ELF relocations, so fixup
> + * .dynamic addresses before proceeding any further. */
> + elf_reloc(image_base, dyn);
> +
> + start64();

Should this call to start64() be moved to your next patch because the
function needs to be fixed and you are fixing it in there ?

> +
> return 0;
> }