* [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-09 19:36 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-09 19:36 UTC (permalink / raw) To: Matt Fleming, Ard Biesheuvel Cc: linux-efi, linux-kernel, Grant Grundler, Greg Hackmann, Michael Davidson, Matthias Kaehlcke, Bernhard Rosenkränzer From: Greg Hackmann <ghackmann@google.com> Without any extra guidance, clang will generate libstub with either absolute or relative ELF relocations. Use the right combination of -fpic and -fno-pic on different files to avoid this. Signed-off-by: Greg Hackmann <ghackmann@google.com> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- drivers/firmware/efi/libstub/Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index f7425960f6a5..ccbaaf4d8650 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ -mno-mmx -mno-sse cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) +ifeq ($(cc-name),clang) +cflags-$(CONFIG_ARM64) += -fpic +endif cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic -mno-single-pic-base @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ $(patsubst %.c,lib-%.o,$(arm-deps)) +ifeq ($(cc-name),clang) +CFLAGS_arm64-stub.o += -fno-pic +endif lib-$(CONFIG_ARM) += arm32-stub.o lib-$(CONFIG_ARM64) += arm64-stub.o -- 2.13.0.rc2.291.g57267f2277-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-09 19:36 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-09 19:36 UTC (permalink / raw) To: Matt Fleming, Ard Biesheuvel Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Greg Hackmann, Michael Davidson, Matthias Kaehlcke, Bernhard Rosenkränzer From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Without any extra guidance, clang will generate libstub with either absolute or relative ELF relocations. Use the right combination of -fpic and -fno-pic on different files to avoid this. Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- drivers/firmware/efi/libstub/Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index f7425960f6a5..ccbaaf4d8650 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ -mno-mmx -mno-sse cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) +ifeq ($(cc-name),clang) +cflags-$(CONFIG_ARM64) += -fpic +endif cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic -mno-single-pic-base @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ $(patsubst %.c,lib-%.o,$(arm-deps)) +ifeq ($(cc-name),clang) +CFLAGS_arm64-stub.o += -fno-pic +endif lib-$(CONFIG_ARM) += arm32-stub.o lib-$(CONFIG_ARM64) += arm64-stub.o -- 2.13.0.rc2.291.g57267f2277-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-09 20:50 ` Greg Hackmann 0 siblings, 0 replies; 21+ messages in thread From: Greg Hackmann @ 2017-05-09 20:50 UTC (permalink / raw) To: Matthias Kaehlcke, Matt Fleming, Ard Biesheuvel Cc: linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > From: Greg Hackmann <ghackmann@google.com> > > Without any extra guidance, clang will generate libstub with either > absolute or relative ELF relocations. Use the right combination of > -fpic and -fno-pic on different files to avoid this. > > Signed-off-by: Greg Hackmann <ghackmann@google.com> > Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > drivers/firmware/efi/libstub/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > index f7425960f6a5..ccbaaf4d8650 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > -mno-mmx -mno-sse > > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > +ifeq ($(cc-name),clang) > +cflags-$(CONFIG_ARM64) += -fpic > +endif > cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > -fno-builtin -fpic -mno-single-pic-base > > @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > > lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > $(patsubst %.c,lib-%.o,$(arm-deps)) > +ifeq ($(cc-name),clang) > +CFLAGS_arm64-stub.o += -fno-pic > +endif > > lib-$(CONFIG_ARM) += arm32-stub.o > lib-$(CONFIG_ARM64) += arm64-stub.o > NAK. This patch was labeled "HACK:" in our experimental tree. There's no rhyme or reason to why this combination of -f[no-]pic flags generates code without problematic relocations. It's inherently fragile, and was only intended as a temporary workaround until I (or someone more familiar with EFI) got a chance to revisit the problem. Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic until the compiler generates what you want", this doesn't belong upstream. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-09 20:50 ` Greg Hackmann 0 siblings, 0 replies; 21+ messages in thread From: Greg Hackmann @ 2017-05-09 20:50 UTC (permalink / raw) To: Matthias Kaehlcke, Matt Fleming, Ard Biesheuvel Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > > Without any extra guidance, clang will generate libstub with either > absolute or relative ELF relocations. Use the right combination of > -fpic and -fno-pic on different files to avoid this. > > Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > --- > drivers/firmware/efi/libstub/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > index f7425960f6a5..ccbaaf4d8650 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > -mno-mmx -mno-sse > > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > +ifeq ($(cc-name),clang) > +cflags-$(CONFIG_ARM64) += -fpic > +endif > cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > -fno-builtin -fpic -mno-single-pic-base > > @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > > lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > $(patsubst %.c,lib-%.o,$(arm-deps)) > +ifeq ($(cc-name),clang) > +CFLAGS_arm64-stub.o += -fno-pic > +endif > > lib-$(CONFIG_ARM) += arm32-stub.o > lib-$(CONFIG_ARM64) += arm64-stub.o > NAK. This patch was labeled "HACK:" in our experimental tree. There's no rhyme or reason to why this combination of -f[no-]pic flags generates code without problematic relocations. It's inherently fragile, and was only intended as a temporary workaround until I (or someone more familiar with EFI) got a chance to revisit the problem. Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic until the compiler generates what you want", this doesn't belong upstream. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 2017-05-09 20:50 ` Greg Hackmann (?) @ 2017-05-09 21:49 ` Matthias Kaehlcke 2017-05-10 7:51 ` Ard Biesheuvel -1 siblings, 1 reply; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-09 21:49 UTC (permalink / raw) To: Greg Hackmann Cc: Matt Fleming, Ard Biesheuvel, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >From: Greg Hackmann <ghackmann@google.com> > > > >Without any extra guidance, clang will generate libstub with either > >absolute or relative ELF relocations. Use the right combination of > >-fpic and -fno-pic on different files to avoid this. > > > >Signed-off-by: Greg Hackmann <ghackmann@google.com> > >Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> > >Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >--- > > drivers/firmware/efi/libstub/Makefile | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > >diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >index f7425960f6a5..ccbaaf4d8650 100644 > >--- a/drivers/firmware/efi/libstub/Makefile > >+++ b/drivers/firmware/efi/libstub/Makefile > >@@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > > -mno-mmx -mno-sse > > > > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >+ifeq ($(cc-name),clang) > >+cflags-$(CONFIG_ARM64) += -fpic > >+endif > > cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > > -fno-builtin -fpic -mno-single-pic-base > > > >@@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > > > > lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > > $(patsubst %.c,lib-%.o,$(arm-deps)) > >+ifeq ($(cc-name),clang) > >+CFLAGS_arm64-stub.o += -fno-pic > >+endif > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > NAK. > > This patch was labeled "HACK:" in our experimental tree. There's no > rhyme or reason to why this combination of -f[no-]pic flags > generates code without problematic relocations. It's inherently > fragile, and was only intended as a temporary workaround until I (or > someone more familiar with EFI) got a chance to revisit the problem. > > Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > until the compiler generates what you want", this doesn't belong > upstream. Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm not very familiar with EFI either. I saw Ard did some work in this code related with relocation, maybe he can provide a pointer towards a better solution. Thanks Matthias ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 2017-05-09 21:49 ` Matthias Kaehlcke @ 2017-05-10 7:51 ` Ard Biesheuvel 2017-05-10 18:38 ` Matthias Kaehlcke 0 siblings, 1 reply; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-10 7:51 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: > El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > >> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: >> >From: Greg Hackmann <ghackmann@google.com> >> > >> >Without any extra guidance, clang will generate libstub with either >> >absolute or relative ELF relocations. Use the right combination of >> >-fpic and -fno-pic on different files to avoid this. >> > >> >Signed-off-by: Greg Hackmann <ghackmann@google.com> >> >Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> >> >Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >> >--- >> > drivers/firmware/efi/libstub/Makefile | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> >diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >> >index f7425960f6a5..ccbaaf4d8650 100644 >> >--- a/drivers/firmware/efi/libstub/Makefile >> >+++ b/drivers/firmware/efi/libstub/Makefile >> >@@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ >> > -mno-mmx -mno-sse >> > >> > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) >> >+ifeq ($(cc-name),clang) >> >+cflags-$(CONFIG_ARM64) += -fpic >> >+endif >> > cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ >> > -fno-builtin -fpic -mno-single-pic-base >> > >> >@@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE >> > >> > lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ >> > $(patsubst %.c,lib-%.o,$(arm-deps)) >> >+ifeq ($(cc-name),clang) >> >+CFLAGS_arm64-stub.o += -fno-pic >> >+endif >> > >> > lib-$(CONFIG_ARM) += arm32-stub.o >> > lib-$(CONFIG_ARM64) += arm64-stub.o >> > >> >> NAK. >> >> This patch was labeled "HACK:" in our experimental tree. There's no >> rhyme or reason to why this combination of -f[no-]pic flags >> generates code without problematic relocations. It's inherently >> fragile, and was only intended as a temporary workaround until I (or >> someone more familiar with EFI) got a chance to revisit the problem. >> >> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic >> until the compiler generates what you want", this doesn't belong >> upstream. > > Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > not very familiar with EFI either. > > I saw Ard did some work in this code related with relocation, maybe he > can provide a pointer towards a better solution. > This is a known issue. The problem is that generic AArch64 small model code is mostly position independent already, due to its use of adrp/add pairs to generate symbol references with a +/- 4 GB range. Building the same code with -fpic will result in GOT entries to be generated, which carry absolute addresses, so this achieves the exact opposite of what we want. The reason for the GOT entries is that GCC (and Clang, apparently) infer from the -fpic flag that you are building objects that will be linked into a shared library, to which ELF symbol preemption rules apply that stipulate that a symbol in the main executable supersedes a symbol under the same name in the shared library, and that the shared library should update all its internal references to the main executable's version of the symbol. The easiest way (but certainly not the only way) to achieve that is to indirect all internal symbol references via GOT entries, which can be made to refer to another symbol by updating a single value. The workaround I used is to use hidden visibility, using a #pragma. (There is a -fvisibility=hidden command line option as well, but this is a weaker form that does not apply to extern declarations, only to definitions). So if you add #pragma GCC visibility push(hidden) at the beginning of arm64-stub.c (and perhaps to one or two other files that contain externally visible symbol declarations these days), you should be able to compile the entire EFI stub with -fpic. Note that making those externally visible symbols 'static' where possible would solve the problem as well, but this triggers another issue in the 32-bit ARM stub. 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 2017-05-10 7:51 ` Ard Biesheuvel @ 2017-05-10 18:38 ` Matthias Kaehlcke 2017-05-10 19:05 ` Ard Biesheuvel 0 siblings, 1 reply; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-10 18:38 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer Hoi Ard, El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: > > El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > > > >> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >> >From: Greg Hackmann <ghackmann@google.com> > >> > > >> >Without any extra guidance, clang will generate libstub with either > >> >absolute or relative ELF relocations. Use the right combination of > >> >-fpic and -fno-pic on different files to avoid this. > >> > > >> >Signed-off-by: Greg Hackmann <ghackmann@google.com> > >> >Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> > >> >Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >> >--- > >> > drivers/firmware/efi/libstub/Makefile | 6 ++++++ > >> > 1 file changed, 6 insertions(+) > >> > > >> >diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >> >index f7425960f6a5..ccbaaf4d8650 100644 > >> >--- a/drivers/firmware/efi/libstub/Makefile > >> >+++ b/drivers/firmware/efi/libstub/Makefile > >> >@@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > >> > -mno-mmx -mno-sse > >> > > >> > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >> >+ifeq ($(cc-name),clang) > >> >+cflags-$(CONFIG_ARM64) += -fpic > >> >+endif > >> > cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > >> > -fno-builtin -fpic -mno-single-pic-base > >> > > >> >@@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > >> > > >> > lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > >> > $(patsubst %.c,lib-%.o,$(arm-deps)) > >> >+ifeq ($(cc-name),clang) > >> >+CFLAGS_arm64-stub.o += -fno-pic > >> >+endif > >> > > >> > lib-$(CONFIG_ARM) += arm32-stub.o > >> > lib-$(CONFIG_ARM64) += arm64-stub.o > >> > > >> > >> NAK. > >> > >> This patch was labeled "HACK:" in our experimental tree. There's no > >> rhyme or reason to why this combination of -f[no-]pic flags > >> generates code without problematic relocations. It's inherently > >> fragile, and was only intended as a temporary workaround until I (or > >> someone more familiar with EFI) got a chance to revisit the problem. > >> > >> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > >> until the compiler generates what you want", this doesn't belong > >> upstream. > > > > Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > > not very familiar with EFI either. > > > > I saw Ard did some work in this code related with relocation, maybe he > > can provide a pointer towards a better solution. > > > > This is a known issue. The problem is that generic AArch64 small model > code is mostly position independent already, due to its use of > adrp/add pairs to generate symbol references with a +/- 4 GB range. > Building the same code with -fpic will result in GOT entries to be > generated, which carry absolute addresses, so this achieves the exact > opposite of what we want. > > The reason for the GOT entries is that GCC (and Clang, apparently) > infer from the -fpic flag that you are building objects that will be > linked into a shared library, to which ELF symbol preemption rules > apply that stipulate that a symbol in the main executable supersedes a > symbol under the same name in the shared library, and that the shared > library should update all its internal references to the main > executable's version of the symbol. The easiest way (but certainly not > the only way) to achieve that is to indirect all internal symbol > references via GOT entries, which can be made to refer to another > symbol by updating a single value. > > The workaround I used is to use hidden visibility, using a #pragma. > (There is a -fvisibility=hidden command line option as well, but this > is a weaker form that does not apply to extern declarations, only to > definitions). So if you add > > #pragma GCC visibility push(hidden) > > at the beginning of arm64-stub.c (and perhaps to one or two other > files that contain externally visible symbol declarations these days), > you should be able to compile the entire EFI stub with -fpic. Note > that making those externally visible symbols 'static' where possible > would solve the problem as well, but this triggers another issue in > the 32-bit ARM stub. > > 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. Cheers Matthias ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-10 19:05 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-10 19:05 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka@chromium.org> wrote: > > Hoi Ard, > > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: >>> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: >>>>> From: Greg Hackmann <ghackmann@google.com> >>>>> >>>>> Without any extra guidance, clang will generate libstub with either >>>>> absolute or relative ELF relocations. Use the right combination of >>>>> -fpic and -fno-pic on different files to avoid this. >>>>> >>>>> Signed-off-by: Greg Hackmann <ghackmann@google.com> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> >>>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >>>>> --- >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ >>>>> 1 file changed, 6 insertions(+) >>>>> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >>>>> index f7425960f6a5..ccbaaf4d8650 100644 >>>>> --- a/drivers/firmware/efi/libstub/Makefile >>>>> +++ b/drivers/firmware/efi/libstub/Makefile >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ >>>>> -mno-mmx -mno-sse >>>>> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) >>>>> +ifeq ($(cc-name),clang) >>>>> +cflags-$(CONFIG_ARM64) += -fpic >>>>> +endif >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ >>>>> -fno-builtin -fpic -mno-single-pic-base >>>>> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE >>>>> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) >>>>> +ifeq ($(cc-name),clang) >>>>> +CFLAGS_arm64-stub.o += -fno-pic >>>>> +endif >>>>> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o >>>>> >>>> >>>> NAK. >>>> >>>> This patch was labeled "HACK:" in our experimental tree. There's no >>>> rhyme or reason to why this combination of -f[no-]pic flags >>>> generates code without problematic relocations. It's inherently >>>> fragile, and was only intended as a temporary workaround until I (or >>>> someone more familiar with EFI) got a chance to revisit the problem. >>>> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic >>>> until the compiler generates what you want", this doesn't belong >>>> upstream. >>> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm >>> not very familiar with EFI either. >>> >>> I saw Ard did some work in this code related with relocation, maybe he >>> can provide a pointer towards a better solution. >>> >> >> This is a known issue. The problem is that generic AArch64 small model >> code is mostly position independent already, due to its use of >> adrp/add pairs to generate symbol references with a +/- 4 GB range. >> Building the same code with -fpic will result in GOT entries to be >> generated, which carry absolute addresses, so this achieves the exact >> opposite of what we want. >> >> The reason for the GOT entries is that GCC (and Clang, apparently) >> infer from the -fpic flag that you are building objects that will be >> linked into a shared library, to which ELF symbol preemption rules >> apply that stipulate that a symbol in the main executable supersedes a >> symbol under the same name in the shared library, and that the shared >> library should update all its internal references to the main >> executable's version of the symbol. The easiest way (but certainly not >> the only way) to achieve that is to indirect all internal symbol >> references via GOT entries, which can be made to refer to another >> symbol by updating a single value. >> >> The workaround I used is to use hidden visibility, using a #pragma. >> (There is a -fvisibility=hidden command line option as well, but this >> is a weaker form that does not apply to extern declarations, only to >> definitions). So if you add >> >> #pragma GCC visibility push(hidden) >> >> at the beginning of arm64-stub.c (and perhaps to one or two other >> files that contain externally visible symbol declarations these days), >> you should be able to compile the entire EFI stub with -fpic. Note >> that making those externally visible symbols 'static' where possible >> would solve the problem as well, but this triggers another issue in >> the 32-bit ARM stub. >> >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-10 19:05 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-10 19:05 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Hackmann, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > > Hoi Ard, > > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: >>> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: >>>>> From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> >>>>> >>>>> Without any extra guidance, clang will generate libstub with either >>>>> absolute or relative ELF relocations. Use the right combination of >>>>> -fpic and -fno-pic on different files to avoid this. >>>>> >>>>> Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> >>>>> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> >>>>> --- >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ >>>>> 1 file changed, 6 insertions(+) >>>>> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >>>>> index f7425960f6a5..ccbaaf4d8650 100644 >>>>> --- a/drivers/firmware/efi/libstub/Makefile >>>>> +++ b/drivers/firmware/efi/libstub/Makefile >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ >>>>> -mno-mmx -mno-sse >>>>> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) >>>>> +ifeq ($(cc-name),clang) >>>>> +cflags-$(CONFIG_ARM64) += -fpic >>>>> +endif >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ >>>>> -fno-builtin -fpic -mno-single-pic-base >>>>> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE >>>>> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) >>>>> +ifeq ($(cc-name),clang) >>>>> +CFLAGS_arm64-stub.o += -fno-pic >>>>> +endif >>>>> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o >>>>> >>>> >>>> NAK. >>>> >>>> This patch was labeled "HACK:" in our experimental tree. There's no >>>> rhyme or reason to why this combination of -f[no-]pic flags >>>> generates code without problematic relocations. It's inherently >>>> fragile, and was only intended as a temporary workaround until I (or >>>> someone more familiar with EFI) got a chance to revisit the problem. >>>> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic >>>> until the compiler generates what you want", this doesn't belong >>>> upstream. >>> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm >>> not very familiar with EFI either. >>> >>> I saw Ard did some work in this code related with relocation, maybe he >>> can provide a pointer towards a better solution. >>> >> >> This is a known issue. The problem is that generic AArch64 small model >> code is mostly position independent already, due to its use of >> adrp/add pairs to generate symbol references with a +/- 4 GB range. >> Building the same code with -fpic will result in GOT entries to be >> generated, which carry absolute addresses, so this achieves the exact >> opposite of what we want. >> >> The reason for the GOT entries is that GCC (and Clang, apparently) >> infer from the -fpic flag that you are building objects that will be >> linked into a shared library, to which ELF symbol preemption rules >> apply that stipulate that a symbol in the main executable supersedes a >> symbol under the same name in the shared library, and that the shared >> library should update all its internal references to the main >> executable's version of the symbol. The easiest way (but certainly not >> the only way) to achieve that is to indirect all internal symbol >> references via GOT entries, which can be made to refer to another >> symbol by updating a single value. >> >> The workaround I used is to use hidden visibility, using a #pragma. >> (There is a -fvisibility=hidden command line option as well, but this >> is a weaker form that does not apply to extern declarations, only to >> definitions). So if you add >> >> #pragma GCC visibility push(hidden) >> >> at the beginning of arm64-stub.c (and perhaps to one or two other >> files that contain externally visible symbol declarations these days), >> you should be able to compile the entire EFI stub with -fpic. Note >> that making those externally visible symbols 'static' where possible >> would solve the problem as well, but this triggers another issue in >> the 32-bit ARM stub. >> >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-10 19:47 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-10 19:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > > > > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka@chromium.org> wrote: > > > > Hoi Ard, > > > > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > > > >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: > >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > >>> > >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >>>>> From: Greg Hackmann <ghackmann@google.com> > >>>>> > >>>>> Without any extra guidance, clang will generate libstub with either > >>>>> absolute or relative ELF relocations. Use the right combination of > >>>>> -fpic and -fno-pic on different files to avoid this. > >>>>> > >>>>> Signed-off-by: Greg Hackmann <ghackmann@google.com> > >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> > >>>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >>>>> --- > >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ > >>>>> 1 file changed, 6 insertions(+) > >>>>> > >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >>>>> index f7425960f6a5..ccbaaf4d8650 100644 > >>>>> --- a/drivers/firmware/efi/libstub/Makefile > >>>>> +++ b/drivers/firmware/efi/libstub/Makefile > >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > >>>>> -mno-mmx -mno-sse > >>>>> > >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >>>>> +ifeq ($(cc-name),clang) > >>>>> +cflags-$(CONFIG_ARM64) += -fpic > >>>>> +endif > >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > >>>>> -fno-builtin -fpic -mno-single-pic-base > >>>>> > >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > >>>>> > >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) > >>>>> +ifeq ($(cc-name),clang) > >>>>> +CFLAGS_arm64-stub.o += -fno-pic > >>>>> +endif > >>>>> > >>>>> lib-$(CONFIG_ARM) += arm32-stub.o > >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o > >>>>> > >>>> > >>>> NAK. > >>>> > >>>> This patch was labeled "HACK:" in our experimental tree. There's no > >>>> rhyme or reason to why this combination of -f[no-]pic flags > >>>> generates code without problematic relocations. It's inherently > >>>> fragile, and was only intended as a temporary workaround until I (or > >>>> someone more familiar with EFI) got a chance to revisit the problem. > >>>> > >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > >>>> until the compiler generates what you want", this doesn't belong > >>>> upstream. > >>> > >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > >>> not very familiar with EFI either. > >>> > >>> I saw Ard did some work in this code related with relocation, maybe he > >>> can provide a pointer towards a better solution. > >>> > >> > >> This is a known issue. The problem is that generic AArch64 small model > >> code is mostly position independent already, due to its use of > >> adrp/add pairs to generate symbol references with a +/- 4 GB range. > >> Building the same code with -fpic will result in GOT entries to be > >> generated, which carry absolute addresses, so this achieves the exact > >> opposite of what we want. > >> > >> The reason for the GOT entries is that GCC (and Clang, apparently) > >> infer from the -fpic flag that you are building objects that will be > >> linked into a shared library, to which ELF symbol preemption rules > >> apply that stipulate that a symbol in the main executable supersedes a > >> symbol under the same name in the shared library, and that the shared > >> library should update all its internal references to the main > >> executable's version of the symbol. The easiest way (but certainly not > >> the only way) to achieve that is to indirect all internal symbol > >> references via GOT entries, which can be made to refer to another > >> symbol by updating a single value. > >> > >> The workaround I used is to use hidden visibility, using a #pragma. > >> (There is a -fvisibility=hidden command line option as well, but this > >> is a weaker form that does not apply to extern declarations, only to > >> definitions). So if you add > >> > >> #pragma GCC visibility push(hidden) > >> > >> at the beginning of arm64-stub.c (and perhaps to one or two other > >> files that contain externally visible symbol declarations these days), > >> you should be able to compile the entire EFI stub with -fpic. Note > >> that making those externally visible symbols 'static' where possible > >> would solve the problem as well, but this triggers another issue in > >> the 32-bit ARM stub. > >> > >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-10 19:47 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-10 19:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > > > > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > > > > Hoi Ard, > > > > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > > > >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > >>> > >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >>>>> From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >>>>> > >>>>> Without any extra guidance, clang will generate libstub with either > >>>>> absolute or relative ELF relocations. Use the right combination of > >>>>> -fpic and -fno-pic on different files to avoid this. > >>>>> > >>>>> Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > >>>>> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > >>>>> --- > >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ > >>>>> 1 file changed, 6 insertions(+) > >>>>> > >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >>>>> index f7425960f6a5..ccbaaf4d8650 100644 > >>>>> --- a/drivers/firmware/efi/libstub/Makefile > >>>>> +++ b/drivers/firmware/efi/libstub/Makefile > >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > >>>>> -mno-mmx -mno-sse > >>>>> > >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >>>>> +ifeq ($(cc-name),clang) > >>>>> +cflags-$(CONFIG_ARM64) += -fpic > >>>>> +endif > >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > >>>>> -fno-builtin -fpic -mno-single-pic-base > >>>>> > >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > >>>>> > >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) > >>>>> +ifeq ($(cc-name),clang) > >>>>> +CFLAGS_arm64-stub.o += -fno-pic > >>>>> +endif > >>>>> > >>>>> lib-$(CONFIG_ARM) += arm32-stub.o > >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o > >>>>> > >>>> > >>>> NAK. > >>>> > >>>> This patch was labeled "HACK:" in our experimental tree. There's no > >>>> rhyme or reason to why this combination of -f[no-]pic flags > >>>> generates code without problematic relocations. It's inherently > >>>> fragile, and was only intended as a temporary workaround until I (or > >>>> someone more familiar with EFI) got a chance to revisit the problem. > >>>> > >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > >>>> until the compiler generates what you want", this doesn't belong > >>>> upstream. > >>> > >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > >>> not very familiar with EFI either. > >>> > >>> I saw Ard did some work in this code related with relocation, maybe he > >>> can provide a pointer towards a better solution. > >>> > >> > >> This is a known issue. The problem is that generic AArch64 small model > >> code is mostly position independent already, due to its use of > >> adrp/add pairs to generate symbol references with a +/- 4 GB range. > >> Building the same code with -fpic will result in GOT entries to be > >> generated, which carry absolute addresses, so this achieves the exact > >> opposite of what we want. > >> > >> The reason for the GOT entries is that GCC (and Clang, apparently) > >> infer from the -fpic flag that you are building objects that will be > >> linked into a shared library, to which ELF symbol preemption rules > >> apply that stipulate that a symbol in the main executable supersedes a > >> symbol under the same name in the shared library, and that the shared > >> library should update all its internal references to the main > >> executable's version of the symbol. The easiest way (but certainly not > >> the only way) to achieve that is to indirect all internal symbol > >> references via GOT entries, which can be made to refer to another > >> symbol by updating a single value. > >> > >> The workaround I used is to use hidden visibility, using a #pragma. > >> (There is a -fvisibility=hidden command line option as well, but this > >> is a weaker form that does not apply to extern declarations, only to > >> definitions). So if you add > >> > >> #pragma GCC visibility push(hidden) > >> > >> at the beginning of arm64-stub.c (and perhaps to one or two other > >> files that contain externally visible symbol declarations these days), > >> you should be able to compile the entire EFI stub with -fpic. Note > >> that making those externally visible symbols 'static' where possible > >> would solve the problem as well, but this triggers another issue in > >> the 32-bit ARM stub. > >> > >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-11 13:51 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-11 13:51 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer (adding Arnd and Will to cc, who are likely to have an opinion as to which GCC is the oldest we need to support for arm64) On 10 May 2017 at 20:47, Matthias Kaehlcke <mka@chromium.org> wrote: > El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > >> >> >> > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka@chromium.org> wrote: >> > >> > Hoi Ard, >> > >> > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: >> > >> >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: >> >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: >> >>> >> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: >> >>>>> From: Greg Hackmann <ghackmann@google.com> >> >>>>> >> >>>>> Without any extra guidance, clang will generate libstub with either >> >>>>> absolute or relative ELF relocations. Use the right combination of >> >>>>> -fpic and -fno-pic on different files to avoid this. >> >>>>> >> >>>>> Signed-off-by: Greg Hackmann <ghackmann@google.com> >> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> >> >>>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >> >>>>> --- >> >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ >> >>>>> 1 file changed, 6 insertions(+) >> >>>>> >> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >> >>>>> index f7425960f6a5..ccbaaf4d8650 100644 >> >>>>> --- a/drivers/firmware/efi/libstub/Makefile >> >>>>> +++ b/drivers/firmware/efi/libstub/Makefile >> >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ >> >>>>> -mno-mmx -mno-sse >> >>>>> >> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) >> >>>>> +ifeq ($(cc-name),clang) >> >>>>> +cflags-$(CONFIG_ARM64) += -fpic >> >>>>> +endif >> >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ >> >>>>> -fno-builtin -fpic -mno-single-pic-base >> >>>>> >> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE >> >>>>> >> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ >> >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) >> >>>>> +ifeq ($(cc-name),clang) >> >>>>> +CFLAGS_arm64-stub.o += -fno-pic >> >>>>> +endif >> >>>>> >> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o >> >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o >> >>>>> >> >>>> >> >>>> NAK. >> >>>> >> >>>> This patch was labeled "HACK:" in our experimental tree. There's no >> >>>> rhyme or reason to why this combination of -f[no-]pic flags >> >>>> generates code without problematic relocations. It's inherently >> >>>> fragile, and was only intended as a temporary workaround until I (or >> >>>> someone more familiar with EFI) got a chance to revisit the problem. >> >>>> >> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic >> >>>> until the compiler generates what you want", this doesn't belong >> >>>> upstream. >> >>> >> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm >> >>> not very familiar with EFI either. >> >>> >> >>> I saw Ard did some work in this code related with relocation, maybe he >> >>> can provide a pointer towards a better solution. >> >>> >> >> >> >> This is a known issue. The problem is that generic AArch64 small model >> >> code is mostly position independent already, due to its use of >> >> adrp/add pairs to generate symbol references with a +/- 4 GB range. >> >> Building the same code with -fpic will result in GOT entries to be >> >> generated, which carry absolute addresses, so this achieves the exact >> >> opposite of what we want. >> >> >> >> The reason for the GOT entries is that GCC (and Clang, apparently) >> >> infer from the -fpic flag that you are building objects that will be >> >> linked into a shared library, to which ELF symbol preemption rules >> >> apply that stipulate that a symbol in the main executable supersedes a >> >> symbol under the same name in the shared library, and that the shared >> >> library should update all its internal references to the main >> >> executable's version of the symbol. The easiest way (but certainly not >> >> the only way) to achieve that is to indirect all internal symbol >> >> references via GOT entries, which can be made to refer to another >> >> symbol by updating a single value. >> >> >> >> The workaround I used is to use hidden visibility, using a #pragma. >> >> (There is a -fvisibility=hidden command line option as well, but this >> >> is a weaker form that does not apply to extern declarations, only to >> >> definitions). So if you add >> >> >> >> #pragma GCC visibility push(hidden) >> >> >> >> at the beginning of arm64-stub.c (and perhaps to one or two other >> >> files that contain externally visible symbol declarations these days), >> >> you should be able to compile the entire EFI stub with -fpic. Note >> >> that making those externally visible symbols 'static' where possible >> >> would solve the problem as well, but this triggers another issue in >> >> the 32-bit ARM stub. >> >> >> >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-11 13:51 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-11 13:51 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Hackmann, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer (adding Arnd and Will to cc, who are likely to have an opinion as to which GCC is the oldest we need to support for arm64) On 10 May 2017 at 20:47, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > >> >> >> > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: >> > >> > Hoi Ard, >> > >> > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: >> > >> >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: >> >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: >> >>> >> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: >> >>>>> From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> >> >>>>> >> >>>>> Without any extra guidance, clang will generate libstub with either >> >>>>> absolute or relative ELF relocations. Use the right combination of >> >>>>> -fpic and -fno-pic on different files to avoid this. >> >>>>> >> >>>>> Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> >> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> >> >>>>> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> >> >>>>> --- >> >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ >> >>>>> 1 file changed, 6 insertions(+) >> >>>>> >> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >> >>>>> index f7425960f6a5..ccbaaf4d8650 100644 >> >>>>> --- a/drivers/firmware/efi/libstub/Makefile >> >>>>> +++ b/drivers/firmware/efi/libstub/Makefile >> >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ >> >>>>> -mno-mmx -mno-sse >> >>>>> >> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) >> >>>>> +ifeq ($(cc-name),clang) >> >>>>> +cflags-$(CONFIG_ARM64) += -fpic >> >>>>> +endif >> >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ >> >>>>> -fno-builtin -fpic -mno-single-pic-base >> >>>>> >> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE >> >>>>> >> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ >> >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) >> >>>>> +ifeq ($(cc-name),clang) >> >>>>> +CFLAGS_arm64-stub.o += -fno-pic >> >>>>> +endif >> >>>>> >> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o >> >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o >> >>>>> >> >>>> >> >>>> NAK. >> >>>> >> >>>> This patch was labeled "HACK:" in our experimental tree. There's no >> >>>> rhyme or reason to why this combination of -f[no-]pic flags >> >>>> generates code without problematic relocations. It's inherently >> >>>> fragile, and was only intended as a temporary workaround until I (or >> >>>> someone more familiar with EFI) got a chance to revisit the problem. >> >>>> >> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic >> >>>> until the compiler generates what you want", this doesn't belong >> >>>> upstream. >> >>> >> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm >> >>> not very familiar with EFI either. >> >>> >> >>> I saw Ard did some work in this code related with relocation, maybe he >> >>> can provide a pointer towards a better solution. >> >>> >> >> >> >> This is a known issue. The problem is that generic AArch64 small model >> >> code is mostly position independent already, due to its use of >> >> adrp/add pairs to generate symbol references with a +/- 4 GB range. >> >> Building the same code with -fpic will result in GOT entries to be >> >> generated, which carry absolute addresses, so this achieves the exact >> >> opposite of what we want. >> >> >> >> The reason for the GOT entries is that GCC (and Clang, apparently) >> >> infer from the -fpic flag that you are building objects that will be >> >> linked into a shared library, to which ELF symbol preemption rules >> >> apply that stipulate that a symbol in the main executable supersedes a >> >> symbol under the same name in the shared library, and that the shared >> >> library should update all its internal references to the main >> >> executable's version of the symbol. The easiest way (but certainly not >> >> the only way) to achieve that is to indirect all internal symbol >> >> references via GOT entries, which can be made to refer to another >> >> symbol by updating a single value. >> >> >> >> The workaround I used is to use hidden visibility, using a #pragma. >> >> (There is a -fvisibility=hidden command line option as well, but this >> >> is a weaker form that does not apply to extern declarations, only to >> >> definitions). So if you add >> >> >> >> #pragma GCC visibility push(hidden) >> >> >> >> at the beginning of arm64-stub.c (and perhaps to one or two other >> >> files that contain externally visible symbol declarations these days), >> >> you should be able to compile the entire EFI stub with -fpic. Note >> >> that making those externally visible symbols 'static' where possible >> >> would solve the problem as well, but this triggers another issue in >> >> the 32-bit ARM stub. >> >> >> >> 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-17 16:09 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-17 16:09 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer El Thu, May 11, 2017 at 02:51:48PM +0100 Ard Biesheuvel ha dit: > (adding Arnd and Will to cc, who are likely to have an opinion as to > which GCC is the oldest we need to support for arm64) > > On 10 May 2017 at 20:47, Matthias Kaehlcke <mka@chromium.org> wrote: > > El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > > > >> > >> > >> > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka@chromium.org> wrote: > >> > > >> > Hoi Ard, > >> > > >> > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > >> > > >> >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka@chromium.org> wrote: > >> >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > >> >>> > >> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >> >>>>> From: Greg Hackmann <ghackmann@google.com> > >> >>>>> > >> >>>>> Without any extra guidance, clang will generate libstub with either > >> >>>>> absolute or relative ELF relocations. Use the right combination of > >> >>>>> -fpic and -fno-pic on different files to avoid this. > >> >>>>> > >> >>>>> Signed-off-by: Greg Hackmann <ghackmann@google.com> > >> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer@linaro.org> > >> >>>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >> >>>>> --- > >> >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ > >> >>>>> 1 file changed, 6 insertions(+) > >> >>>>> > >> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >> >>>>> index f7425960f6a5..ccbaaf4d8650 100644 > >> >>>>> --- a/drivers/firmware/efi/libstub/Makefile > >> >>>>> +++ b/drivers/firmware/efi/libstub/Makefile > >> >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > >> >>>>> -mno-mmx -mno-sse > >> >>>>> > >> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >> >>>>> +ifeq ($(cc-name),clang) > >> >>>>> +cflags-$(CONFIG_ARM64) += -fpic > >> >>>>> +endif > >> >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > >> >>>>> -fno-builtin -fpic -mno-single-pic-base > >> >>>>> > >> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > >> >>>>> > >> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > >> >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) > >> >>>>> +ifeq ($(cc-name),clang) > >> >>>>> +CFLAGS_arm64-stub.o += -fno-pic > >> >>>>> +endif > >> >>>>> > >> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o > >> >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o > >> >>>>> > >> >>>> > >> >>>> NAK. > >> >>>> > >> >>>> This patch was labeled "HACK:" in our experimental tree. There's no > >> >>>> rhyme or reason to why this combination of -f[no-]pic flags > >> >>>> generates code without problematic relocations. It's inherently > >> >>>> fragile, and was only intended as a temporary workaround until I (or > >> >>>> someone more familiar with EFI) got a chance to revisit the problem. > >> >>>> > >> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > >> >>>> until the compiler generates what you want", this doesn't belong > >> >>>> upstream. > >> >>> > >> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > >> >>> not very familiar with EFI either. > >> >>> > >> >>> I saw Ard did some work in this code related with relocation, maybe he > >> >>> can provide a pointer towards a better solution. > >> >>> > >> >> > >> >> This is a known issue. The problem is that generic AArch64 small model > >> >> code is mostly position independent already, due to its use of > >> >> adrp/add pairs to generate symbol references with a +/- 4 GB range. > >> >> Building the same code with -fpic will result in GOT entries to be > >> >> generated, which carry absolute addresses, so this achieves the exact > >> >> opposite of what we want. > >> >> > >> >> The reason for the GOT entries is that GCC (and Clang, apparently) > >> >> infer from the -fpic flag that you are building objects that will be > >> >> linked into a shared library, to which ELF symbol preemption rules > >> >> apply that stipulate that a symbol in the main executable supersedes a > >> >> symbol under the same name in the shared library, and that the shared > >> >> library should update all its internal references to the main > >> >> executable's version of the symbol. The easiest way (but certainly not > >> >> the only way) to achieve that is to indirect all internal symbol > >> >> references via GOT entries, which can be made to refer to another > >> >> symbol by updating a single value. > >> >> > >> >> The workaround I used is to use hidden visibility, using a #pragma. > >> >> (There is a -fvisibility=hidden command line option as well, but this > >> >> is a weaker form that does not apply to extern declarations, only to > >> >> definitions). So if you add > >> >> > >> >> #pragma GCC visibility push(hidden) > >> >> > >> >> at the beginning of arm64-stub.c (and perhaps to one or two other > >> >> files that contain externally visible symbol declarations these days), > >> >> you should be able to compile the entire EFI stub with -fpic. Note > >> >> that making those externally visible symbols 'static' where possible > >> >> would solve the problem as well, but this triggers another issue in > >> >> the 32-bit ARM stub. > >> >> > >> >> 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. Thanks, makes perfect sense. > Arnd, Will, what is the oldest GCC version we claim to support for arm64? Any comments on this Arnd/Will? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-17 16:09 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-17 16:09 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer El Thu, May 11, 2017 at 02:51:48PM +0100 Ard Biesheuvel ha dit: > (adding Arnd and Will to cc, who are likely to have an opinion as to > which GCC is the oldest we need to support for arm64) > > On 10 May 2017 at 20:47, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > > El Wed, May 10, 2017 at 09:05:28PM +0200 Ard Biesheuvel ha dit: > > > >> > >> > >> > On 10 May 2017, at 20:38, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > >> > > >> > Hoi Ard, > >> > > >> > El Wed, May 10, 2017 at 08:51:44AM +0100 Ard Biesheuvel ha dit: > >> > > >> >> On 9 May 2017 at 22:49, Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > >> >>> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit: > >> >>> > >> >>>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote: > >> >>>>> From: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >> >>>>> > >> >>>>> Without any extra guidance, clang will generate libstub with either > >> >>>>> absolute or relative ELF relocations. Use the right combination of > >> >>>>> -fpic and -fno-pic on different files to avoid this. > >> >>>>> > >> >>>>> Signed-off-by: Greg Hackmann <ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >> >>>>> Signed-off-by: Bernhard Rosenkränzer <Bernhard.Rosenkranzer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > >> >>>>> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > >> >>>>> --- > >> >>>>> drivers/firmware/efi/libstub/Makefile | 6 ++++++ > >> >>>>> 1 file changed, 6 insertions(+) > >> >>>>> > >> >>>>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > >> >>>>> index f7425960f6a5..ccbaaf4d8650 100644 > >> >>>>> --- a/drivers/firmware/efi/libstub/Makefile > >> >>>>> +++ b/drivers/firmware/efi/libstub/Makefile > >> >>>>> @@ -11,6 +11,9 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ > >> >>>>> -mno-mmx -mno-sse > >> >>>>> > >> >>>>> cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) > >> >>>>> +ifeq ($(cc-name),clang) > >> >>>>> +cflags-$(CONFIG_ARM64) += -fpic > >> >>>>> +endif > >> >>>>> cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > >> >>>>> -fno-builtin -fpic -mno-single-pic-base > >> >>>>> > >> >>>>> @@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE > >> >>>>> > >> >>>>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \ > >> >>>>> $(patsubst %.c,lib-%.o,$(arm-deps)) > >> >>>>> +ifeq ($(cc-name),clang) > >> >>>>> +CFLAGS_arm64-stub.o += -fno-pic > >> >>>>> +endif > >> >>>>> > >> >>>>> lib-$(CONFIG_ARM) += arm32-stub.o > >> >>>>> lib-$(CONFIG_ARM64) += arm64-stub.o > >> >>>>> > >> >>>> > >> >>>> NAK. > >> >>>> > >> >>>> This patch was labeled "HACK:" in our experimental tree. There's no > >> >>>> rhyme or reason to why this combination of -f[no-]pic flags > >> >>>> generates code without problematic relocations. It's inherently > >> >>>> fragile, and was only intended as a temporary workaround until I (or > >> >>>> someone more familiar with EFI) got a chance to revisit the problem. > >> >>>> > >> >>>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic > >> >>>> until the compiler generates what you want", this doesn't belong > >> >>>> upstream. > >> >>> > >> >>> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm > >> >>> not very familiar with EFI either. > >> >>> > >> >>> I saw Ard did some work in this code related with relocation, maybe he > >> >>> can provide a pointer towards a better solution. > >> >>> > >> >> > >> >> This is a known issue. The problem is that generic AArch64 small model > >> >> code is mostly position independent already, due to its use of > >> >> adrp/add pairs to generate symbol references with a +/- 4 GB range. > >> >> Building the same code with -fpic will result in GOT entries to be > >> >> generated, which carry absolute addresses, so this achieves the exact > >> >> opposite of what we want. > >> >> > >> >> The reason for the GOT entries is that GCC (and Clang, apparently) > >> >> infer from the -fpic flag that you are building objects that will be > >> >> linked into a shared library, to which ELF symbol preemption rules > >> >> apply that stipulate that a symbol in the main executable supersedes a > >> >> symbol under the same name in the shared library, and that the shared > >> >> library should update all its internal references to the main > >> >> executable's version of the symbol. The easiest way (but certainly not > >> >> the only way) to achieve that is to indirect all internal symbol > >> >> references via GOT entries, which can be made to refer to another > >> >> symbol by updating a single value. > >> >> > >> >> The workaround I used is to use hidden visibility, using a #pragma. > >> >> (There is a -fvisibility=hidden command line option as well, but this > >> >> is a weaker form that does not apply to extern declarations, only to > >> >> definitions). So if you add > >> >> > >> >> #pragma GCC visibility push(hidden) > >> >> > >> >> at the beginning of arm64-stub.c (and perhaps to one or two other > >> >> files that contain externally visible symbol declarations these days), > >> >> you should be able to compile the entire EFI stub with -fpic. Note > >> >> that making those externally visible symbols 'static' where possible > >> >> would solve the problem as well, but this triggers another issue in > >> >> the 32-bit ARM stub. > >> >> > >> >> 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. Thanks, makes perfect sense. > Arnd, Will, what is the oldest GCC version we claim to support for arm64? Any comments on this Arnd/Will? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-17 23:24 ` Greg Hackmann 0 siblings, 0 replies; 21+ messages in thread From: Greg Hackmann @ 2017-05-17 23:24 UTC (permalink / raw) To: Ard Biesheuvel, Matthias Kaehlcke Cc: Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-17 23:24 ` Greg Hackmann 0 siblings, 0 replies; 21+ messages in thread From: Greg Hackmann @ 2017-05-17 23:24 UTC (permalink / raw) To: Ard Biesheuvel, Matthias Kaehlcke Cc: Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-18 7:41 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-18 7:41 UTC (permalink / raw) To: Greg Hackmann Cc: Matthias Kaehlcke, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-18 7:41 ` Ard Biesheuvel 0 siblings, 0 replies; 21+ messages in thread From: Ard Biesheuvel @ 2017-05-18 7:41 UTC (permalink / raw) To: Greg Hackmann Cc: Matthias Kaehlcke, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-18 17:00 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-18 17:00 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi, linux-kernel, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64 @ 2017-05-18 17:00 ` Matthias Kaehlcke 0 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2017-05-18 17:00 UTC (permalink / raw) To: Ard Biesheuvel Cc: Greg Hackmann, Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Grundler, Michael Davidson, Bernhard Rosenkränzer 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2017-05-18 17:00 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2017-05-18 17:00 ` Matthias Kaehlcke
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.