Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On Thu, Aug 06, 2020 at 02:19:53PM +0300, Andy Shevchenko wrote: > On Tue, May 26, 2020 at 03:14:11PM -0400, Arvind Sankar wrote: > > Side question: are you going to submit a v3 of this? > Or i.o.w. what is the status of this series? > > -- > With Best Regards, > Andy Shevchenko > > The latest is v6 [0], which has some minor changes over this version and is rebased on 5.8-rc7. I will send out another rebase once the merge window closes. Thanks. [0] https://lore.kernel.org/lkml/20200731202738.2577854-1-nived...@alum.mit.edu/
Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On Tue, May 26, 2020 at 03:14:11PM -0400, Arvind Sankar wrote: Side question: are you going to submit a v3 of this? Or i.o.w. what is the status of this series? -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On Tue, May 26, 2020 at 10:13:40AM -0700, Fangrui Song wrote: > > On 2020-05-26, Arvind Sankar wrote: > >On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote: > >> On Tue, 26 May 2020 at 00:59, Arvind Sankar wrote: > >> > # Compressed kernel should be built as PIE since it may be loaded at any > >> > # address by the bootloader. > >> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, > >> > --no-dynamic-linker) > >> > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker) > >> > >> Do we still need -pie linking with these changes applied? > >> > > > >I think it's currently not strictly necessary -- eg the 64bit kernel > >doesn't get linked as pie right now with LLD or old binutils. However, > >it is safer to do so to ensure that the result remains PIC with future > >versions of the linker. There are linker optimizations that can convert > >certain PIC instructions when PIE is disabled. While I think they > >currently all focus on eliminating indirection through the GOT (and thus > >wouldn't be applicable any more), > > There are 3 forms described by x86-64 psABI B.2 Optimize GOTPCRELX Relocations > > (1) movq foo@GOTPCREL(%rip), %reg -> leaq foo(%rip), %reg > (2) call *foo@GOTPCREL(%rip) -> nop; call foo > (3) jmp *foo@GOTPCREL(%rip) -> jmp foo; nop > > ld.bfd and gold perform (1) even for R_X86_64_GOTPCREL. LLD requires > R_X86_64_[REX_]GOTPCRELX The psABI says (1) can be relaxed into mov $foo, %reg if PIC is disabled and foo lives below 4Gb. Similarly for the "test and binop" cases. Such a relaxation would produce code that's not PIC any more, and wouldn't boot. > > >it's easy to imagine that they could > >get extended to, for eg, convert > > leaqfoo(%rip), %rax > >to > > movl$foo, %eax > >with some nop padding, etc. > > Not with NOP padding, but probably with instruction prefixes. It is > unclear the rewriting will be beneficial. Rewriting instructions definitely > requires a > dedicated relocation type like R_X86_64_[REX_]GOTPCRELX. It ought to be faster: according to Agner Fog's tables, upto 4x higher throughput than the RIP-relative LEA, and movq $foo, %rax is actually the same size. To take a step back, there isn't any *point* in not specifying -pie after these changes: it would be lying to the toolchain just for the sake of lying. It is inherently fragile, and would work only because the toolchain isn't sophisticated enough to do some optimizations. Eg, consider that if you ask for the address of an external function, the compiler will generate movq f@GOTPCREL(%rip), %reg if f has default visibility, and leaq f(%rip), %reg if f has hidden visibility. If you then link without -pie, the former gets relaxed into the non-PIC movq $f, %reg by the BFD linker, but the latter isn't relaxed. This is a missed optimization, which happens because there's the GOTPCRELX to tell the linker that the first form can be relaxed, and there's no special relaxable relocation type for the second form. The 64-bit kernel actually contains one of these relocations, prior to Ard's patches to add hiddden visibility for everything. It currently works with LLD (which can't use -pie) only because LLD doesn't appear to perform the relaxation of movq f@GOTPCREL(%rip), %reg all the way to movq $f, %reg Binutils-2.34, which does do that relaxation, produces an unbootable kernel if you leave out the -pie. > > >Also, the relocation check that's being added here would only work with > >PIE linking.
Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On 2020-05-26, Arvind Sankar wrote: On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote: On Tue, 26 May 2020 at 00:59, Arvind Sankar wrote: > # Compressed kernel should be built as PIE since it may be loaded at any > # address by the bootloader. > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker) > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker) Do we still need -pie linking with these changes applied? I think it's currently not strictly necessary -- eg the 64bit kernel doesn't get linked as pie right now with LLD or old binutils. However, it is safer to do so to ensure that the result remains PIC with future versions of the linker. There are linker optimizations that can convert certain PIC instructions when PIE is disabled. While I think they currently all focus on eliminating indirection through the GOT (and thus wouldn't be applicable any more), There are 3 forms described by x86-64 psABI B.2 Optimize GOTPCRELX Relocations (1) movq foo@GOTPCREL(%rip), %reg -> leaq foo(%rip), %reg (2) call *foo@GOTPCREL(%rip) -> nop; call foo (3) jmp *foo@GOTPCREL(%rip) -> jmp foo; nop ld.bfd and gold perform (1) even for R_X86_64_GOTPCREL. LLD requires R_X86_64_[REX_]GOTPCRELX it's easy to imagine that they could get extended to, for eg, convert leaqfoo(%rip), %rax to movl$foo, %eax with some nop padding, etc. Not with NOP padding, but probably with instruction prefixes. It is unclear the rewriting will be beneficial. Rewriting instructions definitely requires a dedicated relocation type like R_X86_64_[REX_]GOTPCRELX. Also, the relocation check that's being added here would only work with PIE linking.
Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote: > On Tue, 26 May 2020 at 00:59, Arvind Sankar wrote: > > # Compressed kernel should be built as PIE since it may be loaded at any > > # address by the bootloader. > > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, > > --no-dynamic-linker) > > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker) > > Do we still need -pie linking with these changes applied? > I think it's currently not strictly necessary -- eg the 64bit kernel doesn't get linked as pie right now with LLD or old binutils. However, it is safer to do so to ensure that the result remains PIC with future versions of the linker. There are linker optimizations that can convert certain PIC instructions when PIE is disabled. While I think they currently all focus on eliminating indirection through the GOT (and thus wouldn't be applicable any more), it's easy to imagine that they could get extended to, for eg, convert leaqfoo(%rip), %rax to movl$foo, %eax with some nop padding, etc. Also, the relocation check that's being added here would only work with PIE linking.
Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
On Tue, 26 May 2020 at 00:59, Arvind Sankar wrote:
>
> Add a linker script check that there are no runtime relocations, and
> remove the old one that tries to check via looking for specially-named
> sections in the object files.
>
> Drop the tests for -fPIE compiler option and -pie linker option, as they
> are available in all supported gcc and binutils versions (as well as
> clang and lld).
>
> Signed-off-by: Arvind Sankar
> ---
> arch/x86/boot/compressed/Makefile | 28 +++---
> arch/x86/boot/compressed/vmlinux.lds.S | 8
> 2 files changed, 11 insertions(+), 25 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/Makefile
> b/arch/x86/boot/compressed/Makefile
> index d3e882e855ee..679a2b383bfe 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz
> vmlinux.bin.bz2 vmlinux.bin.lzma \
> vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
>
> KBUILD_CFLAGS := -m$(BITS) -O2
> -KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
> +KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> cflags-$(CONFIG_X86_32) := -march=i386
> cflags-$(CONFIG_X86_64) := -mcmodel=small
> @@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
> KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
> # Compressed kernel should be built as PIE since it may be loaded at any
> # address by the bootloader.
> -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option,
> --no-dynamic-linker)
> +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
Do we still need -pie linking with these changes applied?
> LDFLAGS_vmlinux := -T
>
> hostprogs := mkpiggy
> @@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
> vmlinux-objs-$(CONFIG_EFI_STUB) +=
> $(objtree)/drivers/firmware/efi/libstub/lib.a
> vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>
> -# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
> -# can place it anywhere in memory and it will still run. However, since
> -# it is executed as-is without any ELF relocation processing performed
> -# (and has already had all relocation sections stripped from the binary),
> -# none of the code can use data relocations (e.g. static assignments of
> -# pointer values), since they will be meaningless at runtime. This check
> -# will refuse to link the vmlinux if any of these relocations are found.
> -quiet_cmd_check_data_rel = DATAREL $@
> -define cmd_check_data_rel
> - for obj in $(filter %.o,$^); do \
> - $(READELF) -S $$obj | grep -qF .rel.local && { \
> - echo "error: $$obj has data relocations!" >&2; \
> - exit 1; \
> - } || true; \
> - done
> -endef
> -
> -# We need to run two commands under "if_changed", so merge them into a
> -# single invocation.
> -quiet_cmd_check-and-link-vmlinux = LD $@
> - cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
> -
> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> - $(call if_changed,check-and-link-vmlinux)
> + $(call if_changed,ld)
>
> OBJCOPYFLAGS_vmlinux.bin := -R .comment -S
> $(obj)/vmlinux.bin: vmlinux FORCE
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
> b/arch/x86/boot/compressed/vmlinux.lds.S
> index d826ab38a8f9..f9902c6ffe29 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -42,6 +42,12 @@ SECTIONS
> *(.rodata.*)
> _erodata = . ;
> }
> + .rel.dyn : {
> + *(.rel.*)
> + }
> + .rela.dyn : {
> + *(.rela.*)
> + }
> .got : {
> *(.got)
> }
> @@ -83,3 +89,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18,
> "Unexpected GOT/PLT en
> #else
> ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT
> entries detected!")
> #endif
> +
> +ASSERT(SIZEOF(.rel.dyn) == 0 && SIZEOF(.rela.dyn) == 0, "Unexpected runtime
> relocations detected!")
> --
> 2.26.2
>
