Re: [PATCH v5 32/36] x86/boot/compressed: Reorganize zero-size section asserts

Arvind Sankar
2020-08-01T01:47:59Z

On Fri, Jul 31, 2020 at 04:08:16PM -0700, Kees Cook wrote:
> For readability, move the zero-sized sections to the end after DISCARDS
> and mark them NOLOAD for good measure.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/x86/boot/compressed/vmlinux.lds.S | 42 +++++++++++++++-----------
> 1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 3c2ee9a5bf43..42dea70a5091 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -42,18 +42,16 @@ SECTIONS
> *(.rodata.*)
> _erodata = . ;
> }
> - .rel.dyn : {
> - *(.rel.*)
> - }
> - .rela.dyn : {
> - *(.rela.*)
> - }
> - .got : {
> - *(.got)
> - }
> .got.plt : {
> *(.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!")
>
> .data : {
> _data = . ;
> @@ -85,13 +83,23 @@ SECTIONS
> ELF_DETAILS
>
> DISCARDS
> -}
>
> -ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
> -#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
> + /*
> + * Sections that should stay zero sized, which is safer to
> + * explicitly check instead of blindly discarding.
> + */
> + .got (NOLOAD) : {
> + *(.got)
> + }
> + ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
>
> -ASSERT(SIZEOF(.rel.dyn) == 0 && SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations detected!")
> + /* ld.lld does not like .rel* sections being made "NOLOAD". */
> + .rel.dyn : {
> + *(.rel.*)
> + }
> + ASSERT(SIZEOF(.rel.dyn) == 0, "Unexpected run-time relocations (.rel) detected!")
> + .rela.dyn : {
> + *(.rela.*)
> + }
> + ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
> +}
> --
> 2.25.1
>

There's no point in marking zero-size sections NOLOAD -- if the ASSERT's
passed, they won't be present in the file at all anyway.

The only section for which there might be a point is .got.plt, which is
non-empty on 32-bit, and only if it is first moved to the end. That
saves a few bytes.