All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Greg Hackmann" <ghackmann@google.com>,
	"Matt Fleming" <matt@codeblueprint.co.uk>,
	"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Grant Grundler" <grundler@chromium.org>,
	"Michael Davidson" <md@google.com>,
	"Bernhard Rosenkränzer" <Bernhard.Rosenkranzer@linaro.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@mail.gmail.com>

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

> On 18 May 2017 at 00:24, Greg Hackmann <ghackmann@google.com> 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.

WARNING: multiple messages have this Message-ID (diff)
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.

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

Thread overview: 21+ 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
2017-05-09 19:36 ` Matthias Kaehlcke
2017-05-09 20:50 ` Greg Hackmann
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
2017-05-10 19:05         ` Ard Biesheuvel
2017-05-10 19:05           ` Ard Biesheuvel
2017-05-10 19:47           ` Matthias Kaehlcke
2017-05-10 19:47             ` Matthias Kaehlcke
2017-05-11 13:51             ` Ard Biesheuvel
2017-05-11 13:51               ` Ard Biesheuvel
2017-05-17 16:09               ` Matthias Kaehlcke
2017-05-17 16:09                 ` Matthias Kaehlcke
2017-05-17 23:24               ` Greg Hackmann
2017-05-17 23:24                 ` Greg Hackmann
2017-05-18  7:41                 ` Ard Biesheuvel
2017-05-18  7:41                   ` Ard Biesheuvel
2017-05-18 17:00                   ` Matthias Kaehlcke [this message]
2017-05-18 17:00                     ` Matthias Kaehlcke

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@chromium.org \
    --cc=Bernhard.Rosenkranzer@linaro.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=ghackmann@google.com \
    --cc=grundler@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=md@google.com \
    /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.