LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: Russell King <linux@armlinux.org.uk>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Eli Friedman <efriedma@quicinc.com>
Subject: Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement
Date: Tue, 23 Jun 2020 17:03:46 -0700
Message-ID: <CAKwvOdmYa6V=W2eupEmHcuF8+479F8XHxm1NAo0s2N=sawbKAw@mail.gmail.com> (raw)
In-Reply-To: <20200622204915.2987555-2-keescook@chromium.org>

On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@chromium.org> wrote:
>
> We don't want to depend on the linker's orphan section placement
> heuristics as these can vary between linkers, and may change between
> versions. All sections need to be explicitly named in the linker
> script.
>
> Specifically, this would have made a recently fixed bug very obvious:
>
> ld: warning: orphan section `.fixup' from `arch/arm/lib/copy_from_user.o' being placed in section `.fixup'
>
> Refactor linker script include file for use in standard and XIP linker
> scripts, as well as in the coming boot linker script changes. Add debug
> sections explicitly. Create ARM_COMMON_DISCARD macro with unneeded
> sections .ARM.attributes, .iplt, .rel.iplt, .igot.plt, and .modinfo.
> Create ARM_STUBS_TEXT macro with missed text stub sections .vfp11_veneer,
> and .v4_bx. Finally enable orphan section warning.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/arm/Makefile                             |  4 ++++
>  .../arm/{kernel => include/asm}/vmlinux.lds.h | 22 ++++++++++++++-----
>  arch/arm/kernel/vmlinux-xip.lds.S             |  5 ++---
>  arch/arm/kernel/vmlinux.lds.S                 |  5 ++---
>  4 files changed, 25 insertions(+), 11 deletions(-)
>  rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (92%)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 59fde2d598d8..e414e3732b3a 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -16,6 +16,10 @@ LDFLAGS_vmlinux      += --be8
>  KBUILD_LDFLAGS_MODULE  += --be8
>  endif
>
> +# We never want expected sections to be placed heuristically by the
> +# linker. All sections should be explicitly named in the linker script.
> +LDFLAGS_vmlinux += --orphan-handling=warn
> +
>  ifeq ($(CONFIG_ARM_MODULE_PLTS),y)
>  KBUILD_LDS_MODULE      += $(srctree)/arch/arm/kernel/module.lds
>  endif
> diff --git a/arch/arm/kernel/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
> similarity index 92%
> rename from arch/arm/kernel/vmlinux.lds.h
> rename to arch/arm/include/asm/vmlinux.lds.h
> index 381a8e105fa5..3d88ea74f4cd 100644
> --- a/arch/arm/kernel/vmlinux.lds.h
> +++ b/arch/arm/include/asm/vmlinux.lds.h
> @@ -1,4 +1,5 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm-generic/vmlinux.lds.h>
>
>  #ifdef CONFIG_HOTPLUG_CPU
>  #define ARM_CPU_DISCARD(x)
> @@ -37,6 +38,13 @@
>                 *(.idmap.text)                                          \
>                 __idmap_text_end = .;                                   \
>
> +#define ARM_COMMON_DISCARD                                             \
> +               *(.ARM.attributes)                                      \

I could have sworn that someone (Eli?) once told me that this section
(.ARM.attributes) is used for disambiguating which ARM version or
which optional extensions were used when compiling, and that without
this section, one would not be able to disassemble 32b ARM precisely.
If that's the case, we might not want to discard it?

In fact, in LLVM, I can see quite a few tests under
llvm/test/MC/ARM/directive-arch-armv*.s that reference
.ARM.attributes.  Looks like `{llvm|arm-linux-gnueabihf}-readelf
--arch-specific` can be used to dump these sections.  Though I also
only see code in LLVM's tree for writing this, not necessarily reading
it.  Only did a cursory scan of
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp.

Otherwise patch LGTM.

> +               *(.iplt) *(.rel.iplt) *(.igot.plt)                      \
> +               *(.modinfo)                                             \
> +               *(.discard)                                             \
> +               *(.discard.*)
> +
>  #define ARM_DISCARD                                                    \
>                 *(.ARM.exidx.exit.text)                                 \
>                 *(.ARM.extab.exit.text)                                 \
> @@ -49,8 +57,14 @@
>                 EXIT_CALL                                               \
>                 ARM_MMU_DISCARD(*(.text.fixup))                         \
>                 ARM_MMU_DISCARD(*(__ex_table))                          \
> -               *(.discard)                                             \
> -               *(.discard.*)
> +               ARM_COMMON_DISCARD
> +
> +#define ARM_STUBS_TEXT                                                 \
> +               *(.gnu.warning)                                         \
> +               *(.glue_7t)                                             \
> +               *(.glue_7)                                              \

This changes the order of .glue_7t relative to .glue_7.  Maybe that
doesn't matter.

> +               *(.vfp11_veneer)                                        \
> +               *(.v4_bx)
>
>  #define ARM_TEXT                                                       \
>                 IDMAP_TEXT                                              \
> @@ -64,9 +78,7 @@
>                 CPUIDLE_TEXT                                            \
>                 LOCK_TEXT                                               \
>                 KPROBES_TEXT                                            \
> -               *(.gnu.warning)                                         \
> -               *(.glue_7)                                              \
> -               *(.glue_7t)                                             \
> +               ARM_STUBS_TEXT                                          \
>                 . = ALIGN(4);                                           \
>                 *(.got)                 /* Global offset table */       \
>                 ARM_CPU_KEEP(PROC_INFO)
> diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
> index 6d2be994ae58..0807f40844a2 100644
> --- a/arch/arm/kernel/vmlinux-xip.lds.S
> +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> @@ -9,15 +9,13 @@
>
>  #include <linux/sizes.h>
>
> -#include <asm-generic/vmlinux.lds.h>
> +#include <asm/vmlinux.lds.h>
>  #include <asm/cache.h>
>  #include <asm/thread_info.h>
>  #include <asm/memory.h>
>  #include <asm/mpu.h>
>  #include <asm/page.h>
>
> -#include "vmlinux.lds.h"
> -
>  OUTPUT_ARCH(arm)
>  ENTRY(stext)
>
> @@ -152,6 +150,7 @@ SECTIONS
>         _end = .;
>
>         STABS_DEBUG
> +       DWARF_DEBUG
>  }
>
>  /*
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 7f24bc08403e..969205f125ca 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -9,15 +9,13 @@
>  #else
>
>  #include <linux/pgtable.h>
> -#include <asm-generic/vmlinux.lds.h>
> +#include <asm/vmlinux.lds.h>
>  #include <asm/cache.h>
>  #include <asm/thread_info.h>
>  #include <asm/memory.h>
>  #include <asm/mpu.h>
>  #include <asm/page.h>
>
> -#include "vmlinux.lds.h"
> -
>  OUTPUT_ARCH(arm)
>  ENTRY(stext)
>
> @@ -151,6 +149,7 @@ SECTIONS
>         _end = .;
>
>         STABS_DEBUG
> +       DWARF_DEBUG
>  }
>
>  #ifdef CONFIG_STRICT_KERNEL_RWX
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 20:49 [PATCH v2 0/2] arm: " Kees Cook
2020-06-22 20:49 ` [PATCH v2 1/2] arm/build: " Kees Cook
2020-06-24  0:03   ` Nick Desaulniers [this message]
2020-06-24 19:43     ` Kees Cook
2020-06-26 21:36     ` Nick Desaulniers
2020-06-26 21:55       ` Kees Cook
2020-06-22 20:49 ` [PATCH v2 2/2] arm/boot: " Kees Cook
2020-06-24  0:08   ` Nick Desaulniers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKwvOdmYa6V=W2eupEmHcuF8+479F8XHxm1NAo0s2N=sawbKAw@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=efriedma@quicinc.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=masahiroy@kernel.org \
    --cc=natechancellor@gmail.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git