linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arvind Sankar <nivedita@alum.mit.edu>
To: Fangrui Song <maskray@google.com>
Cc: Arvind Sankar <nivedita@alum.mit.edu>,
	Ard Biesheuvel <ardb@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>, X86 ML <x86@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Dmitry Golovin <dima@golovin.in>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations
Date: Tue, 26 May 2020 15:14:11 -0400	[thread overview]
Message-ID: <20200526191411.GA2380966@rani.riverdale.lan> (raw)
In-Reply-To: <20200526171340.pdbautbix5ygdvgp@google.com>

On Tue, May 26, 2020 at 10:13:40AM -0700, Fangrui Song wrote:
> 
> On 2020-05-26, Arvind Sankar wrote:
> >On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote:
> >> On Tue, 26 May 2020 at 00:59, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >> >  # Compressed kernel should be built as PIE since it may be loaded at any
> >> >  # address by the bootloader.
> >> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> >> > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
> >>
> >> Do we still need -pie linking with these changes applied?
> >>
> >
> >I think it's currently not strictly necessary -- eg the 64bit kernel
> >doesn't get linked as pie right now with LLD or old binutils. However,
> >it is safer to do so to ensure that the result remains PIC with future
> >versions of the linker. There are linker optimizations that can convert
> >certain PIC instructions when PIE is disabled. While I think they
> >currently all focus on eliminating indirection through the GOT (and thus
> >wouldn't be applicable any more),
> 
> There are 3 forms described by x86-64 psABI B.2 Optimize GOTPCRELX Relocations
> 
> (1) movq foo@GOTPCREL(%rip), %reg -> leaq foo(%rip), %reg
> (2) call *foo@GOTPCREL(%rip) -> nop; call foo
> (3) jmp *foo@GOTPCREL(%rip) -> jmp foo; nop
> 
> ld.bfd and gold perform (1) even for R_X86_64_GOTPCREL. LLD requires R_X86_64_[REX_]GOTPCRELX

The psABI says (1) can be relaxed into mov $foo, %reg if PIC is disabled
and foo lives below 4Gb. Similarly for the "test and binop" cases. Such
a relaxation would produce code that's not PIC any more, and wouldn't
boot.

> 
> >it's easy to imagine that they could
> >get extended to, for eg, convert
> >	leaq	foo(%rip), %rax
> >to
> >	movl	$foo, %eax
> >with some nop padding, etc.
> 
> Not with NOP padding, but probably with instruction prefixes. It is
> unclear the rewriting will be beneficial. Rewriting instructions definitely requires a
> dedicated relocation type like R_X86_64_[REX_]GOTPCRELX.

It ought to be faster: according to Agner Fog's tables, upto 4x higher
throughput than the RIP-relative LEA, and movq $foo, %rax is actually
the same size.

To take a step back, there isn't any *point* in not specifying -pie
after these changes: it would be lying to the toolchain just for the
sake of lying. It is inherently fragile, and would work only because the
toolchain isn't sophisticated enough to do some optimizations.

Eg, consider that if you ask for the address of an external function,
the compiler will generate
	movq f@GOTPCREL(%rip), %reg
if f has default visibility, and
	leaq f(%rip), %reg
if f has hidden visibility.

If you then link without -pie, the former gets relaxed into the non-PIC
	movq $f, %reg
by the BFD linker, but the latter isn't relaxed. This is a missed
optimization, which happens because there's the GOTPCRELX to tell the
linker that the first form can be relaxed, and there's no special
relaxable relocation type for the second form.

The 64-bit kernel actually contains one of these relocations, prior to
Ard's patches to add hiddden visibility for everything. It currently
works with LLD (which can't use -pie) only because LLD doesn't appear to
perform the relaxation of
	movq f@GOTPCREL(%rip), %reg
all the way to
	movq $f, %reg
Binutils-2.34, which does do that relaxation, produces an unbootable
kernel if you leave out the -pie.

> 
> >Also, the relocation check that's being added here would only work with
> >PIE linking.

  reply	other threads:[~2020-05-26 19:17 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01  8:42 [PATCH] x86/boot: allow a relocatable kernel to be linked with lld Dmitry Golovin
2020-05-02  3:43 ` Nathan Chancellor
2020-05-15 18:50 ` Borislav Petkov
     [not found]   ` <602331589572661@mail.yandex.ru>
2020-05-17 19:44     ` Fangrui Song
2020-05-17 20:25       ` Arvind Sankar
2020-05-18 19:10         ` Nick Desaulniers
2020-05-24 21:28           ` [PATCH 0/4] x86/boot: Remove runtime relocations from compressed kernel Arvind Sankar
2020-05-25  7:10             ` Ard Biesheuvel
2020-05-25 22:59             ` [PATCH v2 " Arvind Sankar
2020-05-26 12:29               ` Sedat Dilek
2020-05-26 12:30                 ` Ard Biesheuvel
2020-05-26 12:33                   ` Sedat Dilek
2020-05-26 12:44                     ` Sedat Dilek
2020-05-26 14:47                       ` Arvind Sankar
2020-05-26 14:50                         ` Sedat Dilek
2020-05-26 15:36                           ` Arvind Sankar
2020-05-26 15:38                             ` Sedat Dilek
2020-05-27  6:26                             ` Sedat Dilek
2020-05-26 14:48                       ` Sedat Dilek
2020-05-26 14:55                         ` Sedat Dilek
2020-05-26 15:07                           ` Sedat Dilek
2020-05-26 15:31                             ` Arvind Sankar
2020-05-27  6:24                               ` Sedat Dilek
2020-05-26 16:18                             ` Sedat Dilek
2020-05-25 22:59             ` [PATCH v2 1/4] x86/boot: Add .text.* to setup.ld Arvind Sankar
2020-05-25 22:59             ` [PATCH v2 2/4] x86/boot: Remove run-time relocations from .head.text code Arvind Sankar
2020-05-25 22:59             ` [PATCH v2 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S Arvind Sankar
2020-05-25 22:59             ` [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations Arvind Sankar
2020-05-26  6:11               ` Ard Biesheuvel
2020-05-26 15:16                 ` Arvind Sankar
2020-05-26 17:13                   ` Fangrui Song
2020-05-26 19:14                     ` Arvind Sankar [this message]
2020-08-06 11:19                       ` Andy Shevchenko
2020-08-06 16:12                         ` Arvind Sankar
2020-05-26  0:37             ` [PATCH 0/4] x86/boot: Remove runtime relocations from compressed kernel Fangrui Song
2020-05-24 21:28           ` [PATCH 1/4] x86/boot: Add .text.startup to setup.ld Arvind Sankar
2020-05-24 22:13             ` Fangrui Song
2020-05-24 23:00               ` Arvind Sankar
2020-05-24 23:49                 ` Fangrui Song
2020-05-24 22:48             ` Brian Gerst
2020-05-24 21:28           ` [PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code Arvind Sankar
2020-05-24 22:53             ` Fangrui Song
2020-05-24 23:44               ` Arvind Sankar
2020-05-25  0:55                 ` Fangrui Song
2020-05-24 21:28           ` [PATCH 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S Arvind Sankar
2020-05-24 23:22             ` Fangrui Song
2020-05-24 23:58               ` Arvind Sankar
2020-05-24 21:28           ` [PATCH 4/4] x86/boot: Check that there are no runtime relocations Arvind Sankar
2020-05-24 23:36             ` Fangrui Song
2020-05-24 23:57               ` Arvind Sankar
2020-05-25  6:10             ` Ard Biesheuvel
2020-05-25 16:26               ` Fangrui Song
2020-05-25 19:22                 ` Arvind Sankar

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=20200526191411.GA2380966@rani.riverdale.lan \
    --to=nivedita@alum.mit.edu \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=daniel.kiper@oracle.com \
    --cc=dima@golovin.in \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=maskray@google.com \
    --cc=mingo@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).