Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64

[Matthias Kaehlcke]

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

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