Messages in this thread | | | Date | Mon, 29 Jun 2020 09:20:31 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v3 7/7] x86/boot: Check that there are no runtime relocations |
| |
On Mon, Jun 29, 2020 at 06:11:59PM +0200, Ard Biesheuvel wrote: > On Mon, 29 Jun 2020 at 18:09, Kees Cook <keescook@chromium.org> wrote: > > > > On Mon, Jun 29, 2020 at 10:09:28AM -0400, 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 <nivedita@alum.mit.edu> > > > Reviewed-by: Ard Biesheuvel <ardb@kernel.org> > > > Reviewed-by: Fangrui Song <maskray@google.com> > > > --- > > > arch/x86/boot/compressed/Makefile | 28 +++----------------------- > > > arch/x86/boot/compressed/vmlinux.lds.S | 8 ++++++++ > > > 2 files changed, 11 insertions(+), 25 deletions(-) > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > question below ... > > > > > diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S > > > index a4a4a59a2628..a78510046eec 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) > > > } > > > > Should these be marked (INFO) as well? > > > > Given that sections marked as (INFO) will still be emitted into the > ELF image, it does not really make a difference to do this for zero > sized sections.
Oh, I misunderstood -- I though they were _not_ emitted; I see now what you said was not allocated. So, disk space used for the .got.plt case, but not memory space used. Sorry for the confusion!
-Kees
> > > > @@ -85,3 +91,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!") > > > > I think I should be doing this same ASSERT style for other explicit > > DISCARDS in my orphan series so we'll notice if they change, instead > > of being silently dropped if they grow. > >
-- Kees Cook
| |