[PATCH 0/3] efi: arm64: use -fpie for building the stub

[PATCH 0/3] efi: arm64: use -fpie for building the stub

[Ard Biesheuvel]

Clang requires the stub to be built with -fpie, or it may emit absolute
symbol references that trigger the absolute relocation detection code and
cause the build to fail.

The remedy is to set -fpie (#3), but due to stupid ELF symbol preemption
rules that only apply to shared libraries, this will result in more rather
than fewer absolute references (i.e., GOT entries), which we have to massage
away using visibility overrides first. (#1, #2)

Ard Biesheuvel (3):
  efi/libstub: arm64: use hidden attribute for struct screen_info
    reference
  efi/libstub: arm64: force 'hidden' visibility for section markers
  efi/libstub: arm64: set -fpie when building the EFI stub

 arch/arm64/include/asm/efi.h              |  3 +++
 drivers/firmware/efi/libstub/Makefile     |  2 +-
 drivers/firmware/efi/libstub/arm64-stub.c | 10 +++++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.9.3

[PATCH 1/3] efi/libstub: arm64: use hidden attribute for struct screen_info reference

[Ard Biesheuvel]

To prevent the compiler from emitting absolute references to screen_info
when building position independent code, redeclare the symbol with hidden
visibility.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/include/asm/efi.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 8f3043aba873..464ac850c5e2 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -81,6 +81,9 @@ static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
 #define alloc_screen_info(x...)		&screen_info
 #define free_screen_info(x...)
 
+/* redeclare as 'hidden' so the compiler will generate relative references */
+extern struct screen_info screen_info __attribute__((__visibility__("hidden")));
+
 static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
 {
 }
-- 
2.9.3

[PATCH 1/3] efi/libstub: arm64: use hidden attribute for struct screen_info reference

[Matthias Kaehlcke]

El Thu, May 18, 2017 at 10:09:51AM +0100 Ard Biesheuvel ha dit:

> To prevent the compiler from emitting absolute references to screen_info
> when building position independent code, redeclare the symbol with hidden
> visibility.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Tested-by: Matthias Kaehlcke <mka@chromium.org>

[PATCH 2/3] efi/libstub: arm64: force 'hidden' visibility for section markers

[Ard Biesheuvel]

To prevent the compiler from emitting absolute references to the section
markers when running in PIC mode, override the visibility to 'hidden' for
all contents of asm/sections.h

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index b4c2589d7c91..f7a6970e9abc 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -9,9 +9,17 @@
  * published by the Free Software Foundation.
  *
  */
+
+/*
+ * To prevent the compiler from emitting GOT-indirected (and thus absolute)
+ * references to the section markers, override their visibility as 'hidden'
+ */
+#pragma GCC visibility push(hidden)
+#include <asm/sections.h>
+#pragma GCC visibility pop
+
 #include <linux/efi.h>
 #include <asm/efi.h>
-#include <asm/sections.h>
 #include <asm/sysreg.h>
 
 #include "efistub.h"
-- 
2.9.3

[PATCH 2/3] efi/libstub: arm64: force 'hidden' visibility for section markers

[Matthias Kaehlcke]

El Thu, May 18, 2017 at 10:09:52AM +0100 Ard Biesheuvel ha dit:

> To prevent the compiler from emitting absolute references to the section
> markers when running in PIC mode, override the visibility to 'hidden' for
> all contents of asm/sections.h
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Tested-by: Matthias Kaehlcke <mka@chromium.org>

[PATCH 3/3] efi/libstub: arm64: set -fpie when building the EFI stub

[Ard Biesheuvel]

Clang may emit absolute symbol references when building in non-PIC mode,
even when using the default 'small' code model, which is already mostly
position independent to begin with, due to its use of adrp/add pairs
that have a relative range of +/- 4 GB. The remedy is to pass the -fpie
flag, which can be done safely now that the code has been updated to avoid
GOT indirections (which may be emitted due to the compiler assuming that
the PIC/PIE code may end up in a shared library that is subject to ELF
symbol preemption)

Passing -fpie when building code that needs to execute at an a priori
unknown offset is arguably an improvement in any case, and given that
the recent visibility changes allow the PIC build to pass with GCC as
well, let's add -fpie for all arm64 builds rather than only for Clang.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index f7425960f6a5..e078390ba477 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -10,7 +10,7 @@ cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
 				   -fPIC -fno-strict-aliasing -mno-red-zone \
 				   -mno-mmx -mno-sse
 
-cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS))
+cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie
 cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic -mno-single-pic-base
 
-- 
2.9.3

[PATCH 3/3] efi/libstub: arm64: set -fpie when building the EFI stub

[Matthias Kaehlcke]

El Thu, May 18, 2017 at 10:09:53AM +0100 Ard Biesheuvel ha dit:

> Clang may emit absolute symbol references when building in non-PIC mode,
> even when using the default 'small' code model, which is already mostly
> position independent to begin with, due to its use of adrp/add pairs
> that have a relative range of +/- 4 GB. The remedy is to pass the -fpie
> flag, which can be done safely now that the code has been updated to avoid
> GOT indirections (which may be emitted due to the compiler assuming that
> the PIC/PIE code may end up in a shared library that is subject to ELF
> symbol preemption)
> 
> Passing -fpie when building code that needs to execute at an a priori
> unknown offset is arguably an improvement in any case, and given that
> the recent visibility changes allow the PIC build to pass with GCC as
> well, let's add -fpie for all arm64 builds rather than only for Clang.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Tested-by: Matthias Kaehlcke <mka@chromium.org>

[PATCH 3/3] efi/libstub: arm64: set -fpie when building the EFI stub

[Ard Biesheuvel]

On 8 June 2017 at 19:08, Matthias Kaehlcke <mka@chromium.org> wrote:
> El Thu, May 18, 2017 at 10:09:53AM +0100 Ard Biesheuvel ha dit:
>
>> Clang may emit absolute symbol references when building in non-PIC mode,
>> even when using the default 'small' code model, which is already mostly
>> position independent to begin with, due to its use of adrp/add pairs
>> that have a relative range of +/- 4 GB. The remedy is to pass the -fpie
>> flag, which can be done safely now that the code has been updated to avoid
>> GOT indirections (which may be emitted due to the compiler assuming that
>> the PIC/PIE code may end up in a shared library that is subject to ELF
>> symbol preemption)
>>
>> Passing -fpie when building code that needs to execute at an a priori
>> unknown offset is arguably an improvement in any case, and given that
>> the recent visibility changes allow the PIC build to pass with GCC as
>> well, let's add -fpie for all arm64 builds rather than only for Clang.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>

Thanks Matthias.

For the record, did you test only with Clang?

[PATCH 3/3] efi/libstub: arm64: set -fpie when building the EFI stub

[Matthias Kaehlcke]

El Thu, Jun 08, 2017 at 07:18:36PM +0000 Ard Biesheuvel ha dit:

> On 8 June 2017 at 19:08, Matthias Kaehlcke <mka@chromium.org> wrote:
> > El Thu, May 18, 2017 at 10:09:53AM +0100 Ard Biesheuvel ha dit:
> >
> >> Clang may emit absolute symbol references when building in non-PIC mode,
> >> even when using the default 'small' code model, which is already mostly
> >> position independent to begin with, due to its use of adrp/add pairs
> >> that have a relative range of +/- 4 GB. The remedy is to pass the -fpie
> >> flag, which can be done safely now that the code has been updated to avoid
> >> GOT indirections (which may be emitted due to the compiler assuming that
> >> the PIC/PIE code may end up in a shared library that is subject to ELF
> >> symbol preemption)
> >>
> >> Passing -fpie when building code that needs to execute at an a priori
> >> unknown offset is arguably an improvement in any case, and given that
> >> the recent visibility changes allow the PIC build to pass with GCC as
> >> well, let's add -fpie for all arm64 builds rather than only for Clang.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > Tested-by: Matthias Kaehlcke <mka@chromium.org>
> 
> Thanks Matthias.
> 
> For the record, did you test only with Clang?

With both gcc and clang. Booting has only been tested with an older
kernel (v3.18), I suppose it shouldn't make a difference.

[PATCH 0/3] efi: arm64: use -fpie for building the stub

[Ard Biesheuvel]

On 18 May 2017 at 10:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Clang requires the stub to be built with -fpie, or it may emit absolute
> symbol references that trigger the absolute relocation detection code and
> cause the build to fail.
>
> The remedy is to set -fpie (#3), but due to stupid ELF symbol preemption
> rules that only apply to shared libraries, this will result in more rather
> than fewer absolute references (i.e., GOT entries), which we have to massage
> away using visibility overrides first. (#1, #2)
>
> Ard Biesheuvel (3):
>   efi/libstub: arm64: use hidden attribute for struct screen_info
>     reference
>   efi/libstub: arm64: force 'hidden' visibility for section markers
>   efi/libstub: arm64: set -fpie when building the EFI stub
>
>  arch/arm64/include/asm/efi.h              |  3 +++
>  drivers/firmware/efi/libstub/Makefile     |  2 +-
>  drivers/firmware/efi/libstub/arm64-stub.c | 10 +++++++++-
>  3 files changed, 13 insertions(+), 2 deletions(-)
>

I have queued these in efi/next, with Matthias's Tested-by