| Message ID | 1475276515-21801-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | b0dddf6c147e6fe61374d625c4bb2b7c52018639 |
| Headers | show |
On 3 October 2016 at 13:52, Matt Fleming <matt@codeblueprint.co.uk> wrote: > On Fri, 30 Sep, at 04:01:55PM, Ard Biesheuvel wrote: >> >> This is a workaround for now. We can revisit this when a need arises to copy >> more kernel code into the stub, by which time we could put in a more elaborate >> fix, or decide to no longer care about 'older' versions of objcopy. >> >> Since this fixes an ARM specific issue and only affects ARM specific Makefile >> variables, I am happy for this to go on top of the arm-soc patch that enables >> CONFIG_EFI for ARM's multi_v7_defconfig (queued for v4.9), given that we have >> no other changes queued in linux-efi that should conflict with this patch. >> >> Matt, any concerns? > > Not with the patch, but could we clarify the user-visible effects of > not applying it? Are the absolute relocations harmless, or will they > lead to crashes? These relocations are harmless, since the debug ones are only interpreted by the debugger, and the ones generated by EXPORT_SYMBOL(sort) will never be referenced, since the symbols they contain are either renamed to __efistub_xxx (arm64), or they are not part of the kernel proper (arm) So both cases are false positives, but the diagnostic is important, and so breaking the build is appropriate for any other absolute relocation that may appear. The effect of the patch is not that the diagnostic is ignored, but that these relocations are not generated in the first place (-g0) or removed explicitly (ksymtab/krcrctab+sort) rather than via a wildcard. So other than not breaking the build, this patch should have no user observeable differences. -- Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 4 October 2016 at 22:30, Matt Fleming <matt@codeblueprint.co.uk> wrote: > On Tue, 04 Oct, at 11:34:31AM, Ard Biesheuvel wrote: >> >> These relocations are harmless, since the debug ones are only >> interpreted by the debugger, and the ones generated by >> EXPORT_SYMBOL(sort) will never be referenced, since the symbols they >> contain are either renamed to __efistub_xxx (arm64), or they are not >> part of the kernel proper (arm) >> >> So both cases are false positives, but the diagnostic is important, >> and so breaking the build is appropriate for any other absolute >> relocation that may appear. >> >> The effect of the patch is not that the diagnostic is ignored, but >> that these relocations are not generated in the first place (-g0) or >> removed explicitly (ksymtab/krcrctab+sort) rather than via a wildcard. >> So other than not breaking the build, this patch should have no user >> observeable differences. > > Thanks Ard, sounds reasonable. Feel free to take this through > whichever tree you think is best. > > Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk> Thanks Matt. Arnd: could you take this on top of the patch that adds CONFIG_EFI to multi_v7_defconfig? That would minimize the breakage, I think. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index c06945160a41..5e23e2d305e7 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -11,7 +11,7 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ $(LINUX_INCLUDE) -O2 \ -mno-mmx -mno-sse cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) -cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ +cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) -g0 \ -fno-builtin -fpic -mno-single-pic-base cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt @@ -79,5 +79,6 @@ quiet_cmd_stubcopy = STUBCPY $@ # decompressor. So move our .data to .data.efistub, which is preserved # explicitly by the decompressor linker script. # -STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub +STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \ + -R ___ksymtab+sort -R ___kcrctab+sort STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
When building the ARM kernel with CONFIG_EFI=y, the following build error may occur when using a less recent version of binutils (2.23 or older): STUBCPY drivers/firmware/efi/libstub/lib-sort.stub.o 00000000 R_ARM_ABS32 sort 00000004 R_ARM_ABS32 __ksymtab_strings drivers/firmware/efi/libstub/lib-sort.stub.o: absolute symbol references not allowed in the EFI stub (and when building with debug symbols, the list above is much longer, and contains all the internal references between the .debug sections and the actual code) This issue is caused by the fact that objcopy v2.23 or earlier does not support wildcards in its -R and -j options, which means the following line from the Makefile: STUBCOPY_FLAGS-y := -R .debug* -R *ksymtab* -R *kcrctab* fails to take effect, leaving harmless absolute relocations in the binary that are indistinguishable from relocations that may cause crashes at runtime due to the fact that these relocations are resolved at link time using the virtual address of the kernel, which is always different from the address at which the EFI firmware loads and invokes the stub. So, as a workaround, disable debug symbols explicitly when building the stub for ARM, and strip the ksymtab and kcrctab symbols for the only exported symbol we currently reuse in the stub, which is 'sort'. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- This is a workaround for now. We can revisit this when a need arises to copy more kernel code into the stub, by which time we could put in a more elaborate fix, or decide to no longer care about 'older' versions of objcopy. Since this fixes an ARM specific issue and only affects ARM specific Makefile variables, I am happy for this to go on top of the arm-soc patch that enables CONFIG_EFI for ARM's multi_v7_defconfig (queued for v4.9), given that we have no other changes queued in linux-efi that should conflict with this patch. Matt, any concerns? drivers/firmware/efi/libstub/Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel