On 15.01.24 18:34, Ard Biesheuvel wrote: > On Sat, 13 Jan 2024 at 11:35, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote: >> >> On 12.01.24 19:56, Palmer Dabbelt wrote: >>> On Fri, 12 Jan 2024 10:51:16 PST (-0800), Ard Biesheuvel wrote: >>>> Hi Jan, >>>> >>>> On Fri, 12 Jan 2024 at 19:37, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote: >>>>> >>>>> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >>>>> >>>>> The cflags for the RISC-V efistub were missing -mno-relax, thus were >>>>> under the risk that the compiler could use GP-relative addressing. That >>>>> happened for _edata with binutils-2.41 and kernel 6.1, causing the >>>>> relocation to fail due to an invalid kernel_size in handle_kernel_image. >>>>> It was not yet observed with newer versions, but that may just be luck. >>>>> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >>>>> --- >>>>> >>>>> Something like this should go to stable as well, but we will need >>>>> rebased patches. >>>>> >>>>> drivers/firmware/efi/libstub/Makefile | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile >>>>> b/drivers/firmware/efi/libstub/Makefile >>>>> index 06964a3c130f..d561d7de46a9 100644 >>>>> --- a/drivers/firmware/efi/libstub/Makefile >>>>> +++ b/drivers/firmware/efi/libstub/Makefile >>>>> @@ -28,7 +28,7 @@ cflags-$(CONFIG_ARM) += -DEFI_HAVE_STRLEN >>>>> -DEFI_HAVE_STRNLEN \ >>>>> -DEFI_HAVE_MEMCHR >>>>> -DEFI_HAVE_STRRCHR \ >>>>> -DEFI_HAVE_STRCMP -fno-builtin >>>>> -fpic \ >>>>> $(call >>>>> cc-option,-mno-single-pic-base) >>>>> -cflags-$(CONFIG_RISCV) += -fpic -DNO_ALTERNATIVE >>>>> +cflags-$(CONFIG_RISCV) += -fpic -DNO_ALTERNATIVE -mno-relax >>>> >>>> Can we detect the presence of these references (via the relocation >>>> type)? We already do something similar for ordinary absolute >>>> references too. >>> >>> If there's no `__global_pointer$` symbol then the linker won't make >>> GP-relative relaxations (because it doesn't know where GP is). We >>> usually define that symbol in the linker script, but I'm not entierly >>> sure how libstub gets its linker script... >>> >> >> The stub seems to be linked together with the rest of the kernel, thus >> the regular arch/riscv/kernel/vmlinux.lds.S is used. >> > > Indeed - the EFI stub is part of the same executable as vmlinux, we > just mangle the symbol names to ensure that only code that can be > safely called from the EFI stub can be linked to it. > > If the effect of -mno-relax is to stop emitting R_RISCV_RELAX > relocations, we should perhaps add those to the STUBCOPY_RELOC-y > Makefile variable? (in the same file). BTW R_RISCV_HI20 doesn't seem > like the right value there to begin with: the idea of that is to > disallow ELF relocations that evaluate to expressions that can only be > known at runtime (like absolute addresses for global pointer > variables) How to do that best? Simply replace R_RISCV_HI20 with R_RISCV_RELAX? Jan -- Siemens AG, Technology Linux Expert Center
