| Message ID | 20200731230820.1742553-30-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Warn on orphan section placement | expand |
On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote: > The .got.plt section should always be zero (or filled only with the > linker-generated lazy dispatch entry). Enforce this with an assert and > mark the section as NOLOAD. This is more sensitive than just blindly > discarding the section. > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index 0cc035cb15f1..7faffe7414d6 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -414,8 +414,20 @@ SECTIONS > ELF_DETAILS > > DISCARDS > -} > > + /* > + * Make sure that the .got.plt is either completely empty or it > + * contains only the lazy dispatch entries. > + */ > + .got.plt (NOLOAD) : { *(.got.plt) } > + ASSERT(SIZEOF(.got.plt) == 0 || > +#ifdef CONFIG_X86_64 > + SIZEOF(.got.plt) == 0x18, > +#else > + SIZEOF(.got.plt) == 0xc, > +#endif > + "Unexpected GOT/PLT entries detected!") > +} > > #ifdef CONFIG_X86_32 > /* > -- > 2.25.1 > Is this actually needed? vmlinux is a position-dependent executable, and it doesn't get linked with any shared libraries, so it should never have a .got or .got.plt at all I think? Does it show up as an orphan without this?
On Fri, Jul 31, 2020 at 10:12:48PM -0400, Arvind Sankar wrote: > On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote: > > The .got.plt section should always be zero (or filled only with the > > linker-generated lazy dispatch entry). Enforce this with an assert and > > mark the section as NOLOAD. This is more sensitive than just blindly > > discarding the section. > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > > index 0cc035cb15f1..7faffe7414d6 100644 > > --- a/arch/x86/kernel/vmlinux.lds.S > > +++ b/arch/x86/kernel/vmlinux.lds.S > > @@ -414,8 +414,20 @@ SECTIONS > > ELF_DETAILS > > > > DISCARDS > > -} > > > > + /* > > + * Make sure that the .got.plt is either completely empty or it > > + * contains only the lazy dispatch entries. > > + */ > > + .got.plt (NOLOAD) : { *(.got.plt) } > > + ASSERT(SIZEOF(.got.plt) == 0 || > > +#ifdef CONFIG_X86_64 > > + SIZEOF(.got.plt) == 0x18, > > +#else > > + SIZEOF(.got.plt) == 0xc, > > +#endif > > + "Unexpected GOT/PLT entries detected!") > > +} > > > > #ifdef CONFIG_X86_32 > > /* > > -- > > 2.25.1 > > > > Is this actually needed? vmlinux is a position-dependent executable, and > it doesn't get linked with any shared libraries, so it should never have > a .got or .got.plt at all I think? Does it show up as an orphan without > this? When I switched from DISCARD to 0-assert, I tested all of these, but given so many combinations, perhaps I made a mistake. I will double-check and report back.
On Fri, Jul 31, 2020 at 10:12:48PM -0400, Arvind Sankar wrote: > On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote: > > The .got.plt section should always be zero (or filled only with the > > linker-generated lazy dispatch entry). Enforce this with an assert and > > mark the section as NOLOAD. This is more sensitive than just blindly > > discarding the section. > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > > index 0cc035cb15f1..7faffe7414d6 100644 > > --- a/arch/x86/kernel/vmlinux.lds.S > > +++ b/arch/x86/kernel/vmlinux.lds.S > > @@ -414,8 +414,20 @@ SECTIONS > > ELF_DETAILS > > > > DISCARDS > > -} > > > > + /* > > + * Make sure that the .got.plt is either completely empty or it > > + * contains only the lazy dispatch entries. > > + */ > > + .got.plt (NOLOAD) : { *(.got.plt) } > > + ASSERT(SIZEOF(.got.plt) == 0 || > > +#ifdef CONFIG_X86_64 > > + SIZEOF(.got.plt) == 0x18, > > +#else > > + SIZEOF(.got.plt) == 0xc, > > +#endif > > + "Unexpected GOT/PLT entries detected!") > > +} > > > > #ifdef CONFIG_X86_32 > > /* > > -- > > 2.25.1 > > > > Is this actually needed? vmlinux is a position-dependent executable, and > it doesn't get linked with any shared libraries, so it should never have > a .got or .got.plt at all I think? Does it show up as an orphan without > this? Yup, I see this: /usr/bin/ld.bfd: warning: orphan section `.got.plt' from `arch/x86/kernel/head_64.o' being placed in section `.got.plt'
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 0cc035cb15f1..7faffe7414d6 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -414,8 +414,20 @@ SECTIONS ELF_DETAILS DISCARDS -} + /* + * Make sure that the .got.plt is either completely empty or it + * contains only the lazy dispatch entries. + */ + .got.plt (NOLOAD) : { *(.got.plt) } + ASSERT(SIZEOF(.got.plt) == 0 || +#ifdef CONFIG_X86_64 + SIZEOF(.got.plt) == 0x18, +#else + SIZEOF(.got.plt) == 0xc, +#endif + "Unexpected GOT/PLT entries detected!") +} #ifdef CONFIG_X86_32 /*
The .got.plt section should always be zero (or filled only with the linker-generated lazy dispatch entry). Enforce this with an assert and mark the section as NOLOAD. This is more sensitive than just blindly discarding the section. Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)