On Mon, Jun 29, 2020 at 10:09:22AM -0400, Arvind Sankar wrote: > From: Ard Biesheuvel <a...@kernel.org> > > The .got.plt section contains the part of the GOT which is used by PLT > entries, and which gets updated lazily by the dynamic loader when > function calls are dispatched through those PLT entries. > > On fully linked binaries such as the kernel proper or the decompressor, > this never happens, and so in practice, the .got.plt section consists > only of the first 3 magic entries that are meant to point at the _DYNAMIC > section and at the fixup routine in the loader. However, since we don't > use a dynamic loader, those entries are never populated or used. > > This means that treating those entries like ordinary GOT entries, and > updating their values based on the actual placement of the executable in > memory is completely pointless, and we can just ignore the .got.plt > section entirely, provided that it has no additional entries beyond > the first 3 ones. > > So add an assertion in the linker script to ensure that this assumption > holds, and move the contents out of the [_got, _egot) memory range that > is modified by the GOT fixup routines. > > While at it, drop the KEEP(), since it has no effect on the contents > of output sections that are created by the linker itself. > > Signed-off-by: Ard Biesheuvel <a...@kernel.org> > Acked-by: Arvind Sankar <nived...@alum.mit.edu> > Signed-off-by: Arvind Sankar <nived...@alum.mit.edu> > From: Ard Biesheuvel <a...@kernel.org> > Link: https://lore.kernel.org/r/20200523120021.34996-2-a...@kernel.org > --- > arch/x86/boot/compressed/vmlinux.lds.S | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/boot/compressed/vmlinux.lds.S > b/arch/x86/boot/compressed/vmlinux.lds.S > index 8f1025d1f681..b17d218ccdf9 100644 > --- a/arch/x86/boot/compressed/vmlinux.lds.S > +++ b/arch/x86/boot/compressed/vmlinux.lds.S > @@ -44,10 +44,13 @@ SECTIONS > } > .got : { > _got = .; > - KEEP(*(.got.plt)) > KEEP(*(.got)) > _egot = .; > } > + .got.plt : { > + *(.got.plt) > + }
This is also being done on arm64, and the section was specified slightly differently (with INFO) which maybe should be done here too? .got.plt (INFO) : { *(.got.plt) } ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, ".got.plt not empty") Otherwise, yes, looks good. Reviewed-by: Kees Cook <keesc...@chromium.org> > + > .data : { > _data = . ; > *(.data) > @@ -77,3 +80,9 @@ SECTIONS > > DISCARDS > } > + > +#ifdef CONFIG_X86_64 > +ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected > GOT/PLT entries detected!") > +#else > +ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT > entries detected!") > +#endif > -- > 2.26.2 > -- Kees Cook