diff mbox series

[v2] efi_loader: Avoid emitting efi_var_buf to .GOT

Message ID 20210116152805.296254-1-ilias.apalodimas@linaro.org
State New
Headers show
Series [v2] efi_loader: Avoid emitting efi_var_buf to .GOT | expand

Commit Message

Ilias Apalodimas Jan. 16, 2021, 3:28 p.m. UTC 
Atish reports that on RISC-V, accessing the EFI variables causes
a kernel panic. An objdump of the file verifies that, since the
global pointer for efi_var_buf ends up in .GOT section which is
not mapped in virtual address space for Linux.

<snip of efi_var_mem_find>

0000000000000084 <efi_var_mem_find>:
  84:   715d                    addi    sp,sp,-80

* objdump -dr
0000000000000086 <.LCFI2>:
  86:   e0a2                    sd  s0,64(sp)
  88:   fc26                    sd  s1,56(sp)
  8a:   e486                    sd  ra,72(sp)
  8c:   f84a                    sd  s2,48(sp)
  8e:   f44e                    sd  s3,40(sp)
  90:   f052                    sd  s4,32(sp)
  92:   ec56                    sd  s5,24(sp)
  94:   00000497            auipc   s1,0x0
            94: R_RISCV_GOT_HI20    efi_var_buf
  98:   0004b483            ld  s1,0(s1) # 94 <.LCFI2+0xe>
            98: R_RISCV_PCREL_LO12_I    .L0
            98: R_RISCV_RELAX   *ABS*

* objdump -t
0000000000000084 g     F .text.efi_runtime  00000000000000b8 efi_var_mem_find

With the patch applied:

* objdump -dr
0000000000000086 <.LCFI2>:
  86:   e0a2                    sd  s0,64(sp)
  88:   fc26                    sd  s1,56(sp)
  8a:   e486                    sd  ra,72(sp)
  8c:   f84a                    sd  s2,48(sp)
  8e:   f44e                    sd  s3,40(sp)
  90:   f052                    sd  s4,32(sp)
  92:   ec56                    sd  s5,24(sp)
  94:   00000497            auipc   s1,0x0
            94: R_RISCV_PCREL_HI20  .LANCHOR0
            94: R_RISCV_RELAX   *ABS*
  98:   00048493            mv  s1,s1
            98: R_RISCV_PCREL_LO12_I    .L0
            98: R_RISCV_RELAX   *ABS*

* objdump -t
0000000000000008 l     O .data.efi_runtime  0000000000000008 efi_var_buf

On arm64 this works, because there's no .GOT entries for this
and everything is converted to relative references.

* objdump -dr (identical pre-post patch, only the new function shows up)
00000000000000b4 <efi_var_mem_find>:
  b4:   aa0003ee    mov x14, x0
  b8:   9000000a    adrp    x10, 0 <efi_var_mem_compare>
            b8: R_AARCH64_ADR_PREL_PG_HI21  .data.efi_runtime
  bc:   91000140    add x0, x10, #0x0
            bc: R_AARCH64_ADD_ABS_LO12_NC   .data.efi_runtime
  c0:   aa0103ed    mov x13, x1
  c4:   79400021    ldrh    w1, [x1]
  c8:   aa0203eb    mov x11, x2
  cc:   f9400400    ldr x0, [x0, #8]
  d0:   b940100c    ldr w12, [x0, #16]
  d4:   8b0c000c    add x12, x0, x12

So let's switch efi_var_buf to static and create a helper function for
anyone that needs to update it.

Fixes: e01aed47d6a0 ("efi_loader: Enable run-time variable support for tee based variables")
Reported-by: Atish Patra <atishp@atishpatra.org>
Tested-by: Atish Patra <atish.patra@wdc.com>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

---
Changes since v1:
- Changed some comments on function and variable declarations
- Added Reviewed-by and Tested-by tags

 include/efi_variable.h            | 11 +++++++++++
 lib/efi_loader/efi_var_mem.c      | 13 ++++++++++++-
 lib/efi_loader/efi_variable_tee.c |  2 +-
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.30.0
diff mbox series

Patch

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4704a3c16e65..89eef1ee5b20 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -306,4 +306,15 @@  efi_status_t __efi_runtime EFIAPI
 efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
 				   u16 *variable_name, efi_guid_t *guid);
 
+/**
+ * efi_var_buf_update() - udpate memory buffer for variables
+ *
+ * @var_buf:	source buffer
+ *
+ * This function copies to the memory buffer for UEFI variables. Call this
+ * function in ExitBootServices() if memory backed variables are only used
+ * at runtime to fill the buffer
+ */
+void efi_var_buf_update(struct efi_var_file *var_buf);
+
 #endif
diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
index d155f25f60e6..bb94fd0da60d 100644
--- a/lib/efi_loader/efi_var_mem.c
+++ b/lib/efi_loader/efi_var_mem.c
@@ -10,7 +10,13 @@ 
 #include <efi_variable.h>
 #include <u-boot/crc.h>
 
-struct efi_var_file __efi_runtime_data *efi_var_buf;
+/*
+ * The variables efi_var_file and efi_var_entry must be static to avoid
+ * referencing them via the global offset table (section .got). The GOT
+ * is neither mapped as EfiRuntimeServicesData nor do we support its
+ * relocation during SetVirtualAddressMap()
+ */
+static struct efi_var_file __efi_runtime_data *efi_var_buf;
 static struct efi_var_entry __efi_runtime_data *efi_current_var;
 
 /**
@@ -339,3 +345,8 @@  efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size,
 
 	return EFI_SUCCESS;
 }
+
+void efi_var_buf_update(struct efi_var_file *var_buf)
+{
+	memcpy(efi_var_buf, var_buf, EFI_VAR_BUF_SIZE);
+}
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index be6f3dfad469..c69330443801 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -692,7 +692,7 @@  void efi_variables_boot_exit_notify(void)
 	if (ret != EFI_SUCCESS)
 		log_err("Can't populate EFI variables. No runtime variables will be available\n");
 	else
-		memcpy(efi_var_buf, var_buf, len);
+		efi_var_buf_update(var_buf);
 	free(var_buf);
 
 	/* Update runtime service table */