Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Arvind Sankar <nivedita@alum.mit.edu>
Date:   Fri, 7 Aug 2020 21:43:27 -0400
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Arvind Sankar <nivedita@alum.mit.edu>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Fangrui Song <maskray@google.com>,
        clang-built-linux <clang-built-linux@googlegroups.com>,
        e5ten.arch@gmail.com, "# 3.4.x" <stable@vger.kernel.org>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Dmitry Golovin <dima@golovin.in>,
        Marco Elver <elver@google.com>, Nick Terrell <terrelln@fb.com>,
        Daniel Kiper <daniel.kiper@oracle.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86/boot: avoid relaxable symbols with Clang
Message-ID: <20200808014327.GA1925552@rani.riverdale.lan>
References: <20200807194100.3570838-1-ndesaulniers@google.com>
 <20200807212914.GB1454138@rani.riverdale.lan>
 <CAKwvOdmD1OMnYE55O+YUkAh+C4Der+2CqKd7JVzfr0+6hYx6jw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAKwvOdmD1OMnYE55O+YUkAh+C4Der+2CqKd7JVzfr0+6hYx6jw@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 07, 2020 at 02:54:39PM -0700, Nick Desaulniers wrote:
> On Fri, Aug 7, 2020 at 2:29 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Fri, Aug 07, 2020 at 12:41:00PM -0700, Nick Desaulniers wrote:
> > > A recent change to a default value of configuration variable
> > > (ENABLE_X86_RELAX_RELOCATIONS OFF -> ON) in LLVM now causes Clang's
> > > integrated assembler to emit R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX
> > > relocations. LLD will relax instructions with these relocations based on
> > > whether the image is being linked as position independent or not.  When
> > > not, then LLD will relax these instructions to use absolute addressing
> > > mode (R_RELAX_GOT_PC_NOPIC). This causes kernels built with Clang
> > > and linked with LLD to fail to boot.
> >
> > It could also cause kernels compiled with gcc and linked with LLD to
> > fail in the same way, no? The gcc/gas combination will generate the
> > relaxed relocations from I think gas-2.26 onward. Although the only
> > troublesome symbol in the case of gcc/gas is trampoline_32bit_src,
> > referenced from pgtable_64.c (gcc doesn't use a GOTPC reloc for _pgtable
> > etc).
> 
> Thanks for taking a look, and the feedback. I appreciate it!
> 
> $ gcc --version | head -n 1
> gcc (Debian 9.3.0-11) 9.3.0
> $ make -j71 clean defconfig bzImage
> $ llvm-readelf -r arch/x86/boot/compressed/*.o | grep -e
> R_X86_64_GOTPCRELX -e R_X86_64_REX_GOTPCRELX
> 0000000000000114  000000120000002a R_X86_64_REX_GOTPCRELX
> 0000000000000000 trampoline_32bit_src - 4
> $ llvm-readelf -r arch/x86/boot/compressed/vmlinux | grep -e
> R_X86_64_GOTPCRELX -e R_X86_64_REX_GOTPCRELX
> $
> 
> So it looks like yes.  I guess then we'd need to add a check for
> CONFIG_LD_IS_LLD and CONFIG_CC_IS_GCC and binutils version is 2.26+?
> I don't mind adding support for that combination, but I'd like to skip
> it in this patch for the sake of backporting something small to stable
> to get our CI green ASAP, since CONFIG_LD_IS_LLD probably doesn't
> exist for those stable branches, which will complicate the backport of
> such a patch.  So I'd do it in a follow up patch if we're cool with
> that?
> 

What if we did it only if we couldn't enable -pie, like the below patch?
I think this should cover all the cases without needing LD_IS_LLD
checks.

For BFD, the only case that should change is binutils-2.26, which
supports relaxations but not -z noreloc-overflow, and will now have
relax-relocations disabled. It currently works (with gcc) only because
the relaxation of
	movq foo@GOTPCREL(%rip), %reg
to
	movq $foo, %reg
in the non-pie case was only added in 2.27, which is also when -z
noreloc-overflow was added, allowing -pie to be enabled. With 2.26, it
only gets relaxed to
	leaq foo(%rip), %reg
which is all LLD currently does as well.

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 8abc30b27ba3..d25bb71f195a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -60,6 +60,13 @@ else
 KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
 	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
 endif
+
+# Disable relocation relaxation if not building as PIE
+ifeq ($(filter -pie,$(KBUILD_LDFLAGS)),)
+KBUILD_CFLAGS += $(call as-option, -Wa$(comma)-mrelax-relocations=no)
+KBUILD_AFLAGS += $(call as-option, -Wa$(comma)-mrelax-relocations=no)
+endif
+
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy