On Mon, 12 Feb 2024 at 11:29, Borislav Petkov <bp@xxxxxxxxx> wrote: > > On Mon, Jan 29, 2024 at 07:05:11PM +0100, Ard Biesheuvel wrote: > > From: Ard Biesheuvel <ardb@xxxxxxxxxx> > > > > Some of the C code in head64.c may be called from a different virtual > > address than it was linked at. Currently, we deal with this by using > > Yeah, make passive pls: "Currently, this is done by using... " > > > ordinary, position dependent codegen, and fixing up all symbol > > references on the fly. This is fragile and tricky to maintain. It is > > also unnecessary: we can use position independent codegen (with hidden > ^^^ > Ditto: "use ..." > > In the comments below too, pls, where it says "we". > Ack. > > visibility) to ensure that all compiler generated symbol references are > > RIP-relative, removing the need for fixups entirely. > > > > It does mean we need explicit references to kernel virtual addresses to > > be generated by hand, so generate those using a movabs instruction in > > inline asm in the handful places where we actually need this. > > > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > > --- > > arch/x86/Makefile | 8 ++ > > arch/x86/boot/compressed/Makefile | 2 +- > > arch/x86/include/asm/desc.h | 3 +- > > arch/x86/include/asm/setup.h | 4 +- > > arch/x86/kernel/Makefile | 5 ++ > > arch/x86/kernel/head64.c | 88 +++++++------------- > > arch/x86/kernel/head_64.S | 5 +- > > 7 files changed, 51 insertions(+), 64 deletions(-) > > > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > > index 1a068de12a56..2b5954e75318 100644 > > --- a/arch/x86/Makefile > > +++ b/arch/x86/Makefile > > @@ -168,6 +168,14 @@ else > > KBUILD_CFLAGS += -mcmodel=kernel > > KBUILD_RUSTFLAGS += -Cno-redzone=y > > KBUILD_RUSTFLAGS += -Ccode-model=kernel > > + > > + PIE_CFLAGS-$(CONFIG_STACKPROTECTOR) += -fno-stack-protector > > Main Makefile has > > KBUILD_CFLAGS += -fno-PIE > > and this ends up being: > > gcc -Wp,-MMD,arch/x86/kernel/.head64.s.d -nostdinc ... -fno-PIE ... -fpie ... -fverbose-asm -S -o arch/x86/kernel/head64.s arch/x86/kernel/head64.c > > Can you pls remove -fno-PIE from those TUs which use PIE_CFLAGS so that > there's no confusion when staring at V=1 output? > Yeah. That would means adding PIE_CFLAGS_REMOVE alongside PIE_CFLAGS and applying both in every place it is used, but we are only dealing with a handful of object files here. > > + PIE_CFLAGS-$(CONFIG_LTO) += -fno-lto > > + > > + PIE_CFLAGS := -fpie -mcmodel=small $(PIE_CFLAGS-y) \ > > + -include $(srctree)/include/linux/hidden.h > > + > > + export PIE_CFLAGS > > endif > > > > # > > Other than that, that code becomes much more readable, cool! > Thanks. But now that we have RIP_REL_REF(), I might split the cleanup from the actual switch to -fpie, which I am still a bit on the fence about, given different compiler versions, LTO, etc. RIP_REL_REF(foo) just turns into 'foo' when compiling with -fpie and we could drop those piecemeal once we are confident that -fpie does not cause any regressions. Note that I have some reservations now about .pi.text as well: it is a bit intrusive, and on x86, we might just as well move everything that executes from the 1:1 mapping into .head.text, and teach objtool that those sections should not contain any ELF relocations involving absolute addresses. But this is another thing that I want to spend a bit more time on before I respin it, so I will just do the cleanup in the next revision, and add the rigid correctness checks the next cycle.
