linux-kernel - Re: [PATCH] arm64: link with -z norelro regardless of CONFIG

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: <CAKwvOd=ZJjYOVubjHN6DFuopMP7jg9PAxGHhOPVu6KefPMNfkg@mail.gmail.com>
Date:   Tue, 20 Oct 2020 13:16:43 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Will Deacon <will@...nel.org>, Ard Biesheuvel <ardb@...nel.org>,
        Fāng-ruì Sòng <maskray@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Peter Oberparleiter <oberpar@...ux.ibm.com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        kernel-team <kernel-team@...roid.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Smith <Peter.Smith@....com>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        "# 3.4.x" <stable@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] arm64: link with -z norelro regardless of CONFIG_RELOCATABLE

On Tue, Oct 20, 2020 at 10:57 AM Will Deacon <will@...nel.org> wrote:
>
> On Fri, 16 Oct 2020 10:53:39 -0700, Nick Desaulniers wrote:
> > With CONFIG_EXPERT=y, CONFIG_KASAN=y, CONFIG_RANDOMIZE_BASE=n,
> > CONFIG_RELOCATABLE=n, we observe the following failure when trying to
> > link the kernel image with LD=ld.lld:
> >
> > error: section: .exit.data is not contiguous with other relro sections
> >
> > ld.lld defaults to -z relro while ld.bfd defaults to -z norelro. This
> > was previously fixed, but only for CONFIG_RELOCATABLE=y.
>
> Applied to arm64 (for-next/core), thanks!
>
> [1/1] arm64: link with -z norelro regardless of CONFIG_RELOCATABLE
>       https://git.kernel.org/arm64/c/3b92fa7485eb

IF we wanted to go further and remove `-z norelro`, or even enable `-z
relro` for aarch64, then we would have to detangle some KASAN/GCOV
generated section discard spaghetti.  Fangrui did some more digging
and found that .fini_array.* sections were relro (read only after
relocations, IIUC), so adding them to EXIT_DATA
(include/asm-generic/vmlinux.lds.h) was causing them to get included
in .exit.data (arch/arm64/kernel/vmlinux.lds.S) making that relro.
There's some history here with commits:

- e41f501d39126 ("vmlinux.lds: account for destructor sections")
- 8dcf86caa1e3da ("vmlinux.lds.h: Fix incomplete .text.exit discards")
- d812db78288d7 ("vmlinux.lds.h: Avoid KASAN and KCSAN's unwanted sections")

It seems the following works for quite a few different
configs/toolchains I played with, but the big IF is whether enabling
`-z relro` is worthwhile?  If the kernel does respect that mapping,
then I assume that's a yes, but I haven't checked yet whether relro is
respected within the kernel (`grep -rn RELRO` turns up nothing
interesting).  I also haven't checked yet whether all supported
versions of GNU ld.bfd support -z relro (guessing not, since a quick
test warns: `aarch64-linux-gnu-ld: warning: -z relro ignored` for
v2.34.90.20200706, may be holding it wrong).

(Fangrui also filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97507
in regards to GCOV+GCC)

diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index cd14444bf600..64578c998e53 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -744,7 +744,6 @@

 #define EXIT_DATA                                                      \
        *(.exit.data .exit.data.*)                                      \
-       *(.fini_array .fini_array.*)                                    \
        *(.dtors .dtors.*)                                              \
        MEM_DISCARD(exit.data*)                                         \
        MEM_DISCARD(exit.rodata*)
@@ -995,6 +994,7 @@
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KCSAN)
 # ifdef CONFIG_CONSTRUCTORS
 #  define SANITIZER_DISCARDS                                           \
+       *(.fini_array .fini_array.*)                                    \
        *(.eh_frame)
 # else
 #  define SANITIZER_DISCARDS                                           \
@@ -1005,8 +1005,16 @@
 # define SANITIZER_DISCARDS
 #endif

+#if defined(CONFIG_GCOV_KERNEL) && defined(CONFIG_CC_IS_GCC)
+# define GCOV_DISCARDS                                                 \
+       *(.fini_array .fini_array.*)
+#else
+# define GCOV_DISCARDS
+#endif
+
 #define COMMON_DISCARDS
         \
        SANITIZER_DISCARDS                                              \
+       GCOV_DISCARDS                                                   \
        *(.discard)                                                     \
        *(.discard.*)                                                   \
        *(.modinfo)                                                     \
-- 
Thanks,
~Nick Desaulniers