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(-)
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 */