All of lore.kernel.org
 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>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Nick Desaulniers <ndesaulniers@google.com>,
	Dmitry Golovin <dima@golovin.in>,
	clang-built-linux@googlegroups.com,
	Ard Biesheuvel <ardb@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code
Date: Sun, 24 May 2020 19:44:02 -0400	[thread overview]
Message-ID: <20200524234402.GB280334@rani.riverdale.lan> (raw)
In-Reply-To: <20200524225359.wnc43jmh6rvfaezq@google.com>

On Sun, May 24, 2020 at 03:53:59PM -0700, Fangrui Song wrote:
> On 2020-05-24, Arvind Sankar wrote:
> >The assembly code in head_{32,64}.S, while meant to be
> >position-independent, generates run-time relocations because it uses
> >instructions such as
> >	leal	gdt(%edx), %eax
> >which make the assembler and linker think that the code is using %edx as
> >an index into gdt, and hence gdt needs to be relocated to its run-time
> >address.
> >
> >With the BFD linker, this generates a warning during the build:
> >  LD      arch/x86/boot/compressed/vmlinux
> >ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
> >ld: warning: creating a DT_TEXTREL in object
> 
> Interesting. How does the build generate a warning by default?
> Do you use Gentoo Linux which appears to ship a --warn-shared-textrel
> enabled-by-default patch? (https://bugs.gentoo.org/700488)

Ah, yes I am using gentoo. I didn't realize that was a distro
modification.

> >+
> >+/*
> >+ * This macro gives the link address of X. It's the same as X, since startup_32
> >+ * has link address 0, but defining it this way tells the assembler/linker that
> >+ * we want the link address, and not the run-time address of X. This prevents
> >+ * the linker from creating a run-time relocation entry for this reference.
> >+ * The macro should be used as a displacement with a base register containing
> >+ * the run-time address of startup_32 [i.e. la(X)(%reg)], or as an
> >+ * immediate [$ la(X)].
> >+ *
> >+ * This macro can only be used from within the .head.text section, since the
> >+ * expression requires startup_32 to be in the same section as the code being
> >+ * assembled.
> >+ */
> >+#define la(X) ((X) - startup_32)
> >+
> 
> IIRC, %ebp contains the address of startup_32. la(X) references X
> relative to startup_32. The fixup (in GNU as and clang integrated
> assembler's term) is a constant which is resolved by the assembler.
> 
> There is no R_386_32 or R_386_PC32 for the linker to resolve.

This is incorrect (or maybe I'm not understanding you correctly). X is a
symbol whose final location relative to startup_32 is in most cases not
known to the assembler (there are a couple of cases where X is a label
within .head.text which do get completely resolved by the assembler).

For example, taking the instruction loading the gdt address, this is
what we get from the assembler:
	lea	la(gdt)(%ebp), %eax
becomes in the object file:
  11:   8d 85 00 00 00 00       lea    0x0(%ebp),%eax
			13: R_X86_64_PC32       .data+0x23
or a cleaner example using a global symbol:
	subl	la(image_offset)(%ebp), %ebx
becomes
  41:   2b 9d 00 00 00 00       sub    0x0(%ebp),%ebx
			43: R_X86_64_PC32       image_offset+0x43

So in general you get PC32 relocations, with the addend being set by the
assembler to .-startup_32, modulo the adjustment for where within the
instruction the displacement needs to be. These relocations are resolved
by the static linker to produce constants in the final executable.


> 
> Not very sure stating that "since startup_32 has link address 0" is very
> appropriate here (probably because I did't see the term "link address"
> before). If my understanding above is correct, I think you can just
> reword the comment to express that X is referenced relative to
> startup_32, which is stored in %ebp.
> 

Yeah, the more standard term is virtual address/VMA, but that sounds
confusing to me with PIE code since the _actual_ virtual address at
which this code is going to run isn't 0, that's just the address assumed
for linking. I can reword it to avoid referencing "link address" but
then it's not obvious why the macro is named "la" :) I'm open to
suggestions on a better name, I could use offset but that's a bit
long-winded. I could just use vma() if nobody else finds it confusing.

Thanks.

  reply	other threads:[~2020-05-24 23:44 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
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 [this message]
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=20200524234402.GB280334@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.