lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKwvOdmYa6V=W2eupEmHcuF8+479F8XHxm1NAo0s2N=sawbKAw@mail.gmail.com>
Date:   Tue, 23 Jun 2020 17:03:46 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Russell King <linux@...linux.org.uk>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        Will Deacon <will@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Eli Friedman <efriedma@...cinc.com>
Subject: Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement

On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@...omium.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@...omium.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ