Re: [PATCH v3 7/7] x86/boot: Check that there are no runtime relocations

[Fangrui Song]

On 2020-06-29, Arvind Sankar wrote:
> On Mon, Jun 29, 2020 at 09:20:31AM -0700, Kees Cook wrote:
> > On Mon, Jun 29, 2020 at 06:11:59PM +0200, Ard Biesheuvel wrote:
> > > On Mon, 29 Jun 2020 at 18:09, Kees Cook <keescook@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > > > Reviewed-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > > > Reviewed-by: Fangrui Song <maskray@xxxxxxxxxx>
> > > > > ---
> > > > >  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@xxxxxxxxxxxx>
> > > >
> > > > 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

About output section type (INFO):
https://sourceware.org/binutils/docs/ld/Output-Section-Type.html#Output-Section-Type
says "These type names are supported for backward compatibility, and are
rarely used."

If all input section don't have the SHF_ALLOC flag, the output section
will not have this flag as well. This type is not useful...

If .got and .got.plt were used, they should be considered dynamic
relocations which should be part of the loadable image. So they should
have the SHF_ALLOC flag. (INFO) will not be applicable anyway.

SHT_REL[A] may be allocable or not. Usually .rel[a].dyn and .rel[a].plt
are linker created allocable sections. (INFO) does not make sense for them.

> In the case of the REL[A] and .got sections, they are actually already
> not emitted at all into the ELF file now that they are zero size.
>
> For .got.plt, it is only emitted for 32-bit (with the 3 reserved
> entries), the 64-bit linker seems to get rid of it.