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

[Fangrui Song]

On 2020-06-29, Ard Biesheuvel wrote:
> On Mon, 29 Jun 2020 at 19:37, Fangrui Song <maskray@xxxxxxxxxx> wrote:
> > 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.
>
> I don't care deeply either way, but Kees indicated that he would like
> to get rid of the 24 bytes of .got.plt magic entries that we have no
> need for.
>
> In fact, a lot of this mangling is caused by the fact that the linker
> is creating a relocatable binary, and assumes that it is a hosted
> binary that is loaded by a dynamic loader. It would actually be much
> better if the compiler and linker would take -ffreestanding into
> account, and suppress GOT entries, PLTs, dynamic program headers for
> shared libraries altogether.

Linkers (GNU ld and LLD) don't create .got or .got.plt just because the linker
command line has -pie or -shared.  They create .got or .got.plt if there are
specific needs.

For .got.plt, if there is (1) any .plt/.iplt entry, (2) any .got.plt based
relocation (e.g. R_X86_64_GOTPC32 on x86-64), or (3) if _GLOBAL_OFFSET_TABLE_ is
referenced, .got.plt will be created (both GNU ld and LLD) with usually 3
entries (for ld.so purposes).

If (1) is not satisfied, the created .got.plt is just served as an anchor for
things that want to reference (the distance from GOT base to some point). The
linker will still reserve 3 words but the words are likely not needed.

I don't think there is a specific need for another option to teach the linker
(GNU ld or LLD) that this is a kernel link.  For -ffreestanding builds, cc
-static (ld -no-pie))/-static-pie (-pie) already work quite well.