linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: "Greg Hackmann"
	<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	"Matt Fleming"
	<matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>,
	"linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"Grant Grundler"
	<grundler-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	"Michael Davidson" <md-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	"Bernhard Rosenkränzer"
	<Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64
Date: Thu, 18 May 2017 10:00:51 -0700	[thread overview]
Message-ID: <20170518170051.GI141096@google.com> (raw)
In-Reply-To: <CAKv+Gu_JE8No4Ob-kL4tubRYiuFWYbptwouw16ezFhkLSizn5w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

El Thu, May 18, 2017 at 08:41:26AM +0100 Ard Biesheuvel ha dit:

> On 18 May 2017 at 00:24, Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> > On 05/11/2017 06:51 AM, Ard Biesheuvel wrote:
> > [snip]
> >>>>>>
> >>>>>> In my opinion, the correct fix would be to make -fpie (as opposed to
> >>>>>> -fpic) imply hidden visibility, given that PIE executables don't
> >>>>>> export symbols in the first place, and so the preemption rules do not
> >>>>>> apply. It is worth a try whether -fpie works as expected in this case
> >>>>>> on Clang, but the last time I tried it on GCC, it behaved exactly like
> >>>>>> -fpic.
> >>>>>
> >>>>>
> >>>>> Thanks a lot for the detailed description and your suggestions!
> >>>>>
> >>>>> A clang build with -fpie for the EFI stub succeeds without complaints
> >>>>> about GOT entries. I will send out an updated patch (with -fpie only
> >>>>> for clang) later.
> >>>>>
> >>>>
> >>>> Good! I never liked the visibility hack, which is why I never upstreamed
> >>>> it.
> >>>>
> >>>> Could you please check how recent GCC behaves?
> >>>
> >>>
> >>> I tried GCC v4.9.4 and v6.3.1, both build the EFI stub with -fpie
> >>> without errors.
> >>>
> >>> Are you suggesting to use -fpie for both clang and GCC? Do you know
> >>> what the minimum required GCC version is for building an arm64 kernel?
> >>
> >>
> >> Yes. Up until now, we have been relying on the position independent
> >> nature of small model code, but it would be better to specify it
> >> explicitly, so if -fpie gives us mostly identical code and does not
> >> need visibility hacks, I would prefer to add it for all compilers and
> >> not have an exception only for Clang. Note that the same applies to
> >> the entire kernel when built in KASLR mode, so it would also be good
> >> to know our options here.
> >>
> >> Arnd, Will, what is the oldest GCC version we claim to support for arm64?
> >>
> >
> > Unfortunately, after looking into this a bit more, -fpie by itself doesn't
> > force clang to disable symbol preeemption.  For example when building the
> > EFI stub from 4.9 with clang, -fpie gives me a stub that crashes with a
> > synchronous exception inside handle_kernel_image().  The faulting
> > instruction is a read from __nokaslr that still goes through the GOT.
> >
> > Right now you'll get a usable EFI stub with -fpie anyway, since 60f38de7a8d4
> > ("efi/libstub: Unify command line param parsing") masked the problem when it
> > moved __nokaslr behind a helper function.  But AIUI there's nothing really
> > preventing a similar problem in the future.
> >
> > You *can* force clang to disable symbol preemption using "-fpie
> > -mpie-copy-relocations".  That said, I don't know enough about EFI to say
> > whether this is actually appropriate for building the EFI stub.

Thanks for the investigation, Greg.

> Thanks for digging into this. It is really quite unfortunate that it
> is so difficult to force Clang (or GCC for that matter) to generate
> relative references without the compiler assuming that you are
> building a shared library. Perhaps we need a stronger version of
> -fvisibility=hidden, i.e., one that applies to extern declarations as
> well.
> 
> For the stub, we could simply replace all remaining extern symbol
> references (if any) with accessor functions, such as the one I added
> for __nokaslr (which is actually needed for x86 as well, for different
> reasons). Let me look into this.

I saw you sent out a bunch of patches, thanks, your help is greatly
appreciated.

      parent reply	other threads:[~2017-05-18 17:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-09 19:36 [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 Matthias Kaehlcke
     [not found] ` <20170509193612.64105-1-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-05-09 20:50   ` Greg Hackmann
2017-05-09 21:49     ` Matthias Kaehlcke
2017-05-10  7:51       ` Ard Biesheuvel
2017-05-10 18:38         ` Matthias Kaehlcke
     [not found]           ` <20170510183848.GI128305-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-05-10 19:05             ` Ard Biesheuvel
     [not found]               ` <69A18345-6933-4C6A-8FAC-DBD4D7EF30DE-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-05-10 19:47                 ` Matthias Kaehlcke
     [not found]                   ` <20170510194734.GJ128305-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-05-11 13:51                     ` Ard Biesheuvel
     [not found]                       ` <CAKv+Gu8H9H=Rax4iiKaZ2z2GikXTjseCAyKrYgQPQWkekjsN8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-17 16:09                         ` Matthias Kaehlcke
2017-05-17 23:24                         ` Greg Hackmann
     [not found]                           ` <a5692a7a-0140-2e61-497f-6d3bf92b5d11-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-05-18  7:41                             ` Ard Biesheuvel
     [not found]                               ` <CAKv+Gu_JE8No4Ob-kL4tubRYiuFWYbptwouw16ezFhkLSizn5w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-18 17:00                                 ` Matthias Kaehlcke [this message]

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=20170518170051.GI141096@google.com \
    --to=mka-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
    --cc=Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=grundler-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org \
    --cc=md-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.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).