| Message ID | 20200624014940.1204448-4-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Warn on orphan section placement | expand |
On 2020-06-23, Kees Cook wrote: >In preparation for adding --orphan-handling=warn to more architectures, >make sure unwanted sections don't end up appearing under the .init >section prefix that libstub adds to itself during objcopy. > >Signed-off-by: Kees Cook <keescook@chromium.org> >--- > drivers/firmware/efi/libstub/Makefile | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile >index 75daaf20374e..9d2d2e784bca 100644 >--- a/drivers/firmware/efi/libstub/Makefile >+++ b/drivers/firmware/efi/libstub/Makefile >@@ -66,6 +66,9 @@ lib-$(CONFIG_X86) += x86-stub.o > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > >+# Remove unwanted sections first. >+STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property >+ > # > # For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the > # .bss section, so the .bss section of the EFI stub needs to be included in the arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti else branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf endif This option creates .note.gnu.property: % readelf -n drivers/firmware/efi/libstub/efi-stub.o Displaying notes found in: .note.gnu.property Owner Data size Description GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 Properties: AArch64 feature: PAC If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf
On Tue, Jun 23, 2020 at 08:31:42PM -0700, 'Fangrui Song' via Clang Built Linux wrote: > On 2020-06-23, Kees Cook wrote: > > In preparation for adding --orphan-handling=warn to more architectures, > > make sure unwanted sections don't end up appearing under the .init > > section prefix that libstub adds to itself during objcopy. > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > drivers/firmware/efi/libstub/Makefile | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > index 75daaf20374e..9d2d2e784bca 100644 > > --- a/drivers/firmware/efi/libstub/Makefile > > +++ b/drivers/firmware/efi/libstub/Makefile > > @@ -66,6 +66,9 @@ lib-$(CONFIG_X86) += x86-stub.o > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > +# Remove unwanted sections first. > > +STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property > > + > > # > > # For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the > > # .bss section, so the .bss section of the EFI stub needs to be included in the > > arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on > > ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti > else > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf > endif > > This option creates .note.gnu.property: > > % readelf -n drivers/firmware/efi/libstub/efi-stub.o > > Displaying notes found in: .note.gnu.property > Owner Data size Description > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > Properties: AArch64 feature: PAC > > If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying > -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf We want to keep the branch protection enabled. But since it's not a "regular" ELF, we don't need to keep the property that identifies the feature.
On Tue, Jun 23, 2020 at 09:44:11PM -0700, Kees Cook wrote: > On Tue, Jun 23, 2020 at 08:31:42PM -0700, 'Fangrui Song' via Clang Built Linux wrote: > > On 2020-06-23, Kees Cook wrote: > > > In preparation for adding --orphan-handling=warn to more architectures, > > > make sure unwanted sections don't end up appearing under the .init > > > section prefix that libstub adds to itself during objcopy. > > > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > --- > > > drivers/firmware/efi/libstub/Makefile | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > index 75daaf20374e..9d2d2e784bca 100644 > > > --- a/drivers/firmware/efi/libstub/Makefile > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > @@ -66,6 +66,9 @@ lib-$(CONFIG_X86) += x86-stub.o > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > +# Remove unwanted sections first. > > > +STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property > > > + > > > # > > > # For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the > > > # .bss section, so the .bss section of the EFI stub needs to be included in the > > > > arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on > > > > ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti > > else > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf > > endif > > > > This option creates .note.gnu.property: > > > > % readelf -n drivers/firmware/efi/libstub/efi-stub.o > > > > Displaying notes found in: .note.gnu.property > > Owner Data size Description > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > Properties: AArch64 feature: PAC > > > > If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying > > -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf > > We want to keep the branch protection enabled. But since it's not a > "regular" ELF, we don't need to keep the property that identifies the > feature. For the kernel Image, how do we remove these sections? The objcopy flags in arch/arm64/boot/Makefile look both insufficient and out of date. My vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" segment. Will
On Wed, 24 Jun 2020 at 12:44, Will Deacon <will@kernel.org> wrote: > > On Tue, Jun 23, 2020 at 09:44:11PM -0700, Kees Cook wrote: > > On Tue, Jun 23, 2020 at 08:31:42PM -0700, 'Fangrui Song' via Clang Built Linux wrote: > > > On 2020-06-23, Kees Cook wrote: > > > > In preparation for adding --orphan-handling=warn to more architectures, > > > > make sure unwanted sections don't end up appearing under the .init > > > > section prefix that libstub adds to itself during objcopy. > > > > > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > > --- > > > > drivers/firmware/efi/libstub/Makefile | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > > index 75daaf20374e..9d2d2e784bca 100644 > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > @@ -66,6 +66,9 @@ lib-$(CONFIG_X86) += x86-stub.o > > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > +# Remove unwanted sections first. > > > > +STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property > > > > + > > > > # > > > > # For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the > > > > # .bss section, so the .bss section of the EFI stub needs to be included in the > > > > > > arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on > > > > > > ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti > > > else > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf > > > endif > > > > > > This option creates .note.gnu.property: > > > > > > % readelf -n drivers/firmware/efi/libstub/efi-stub.o > > > > > > Displaying notes found in: .note.gnu.property > > > Owner Data size Description > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > Properties: AArch64 feature: PAC > > > > > > If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying > > > -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf > > > > We want to keep the branch protection enabled. But since it's not a > > "regular" ELF, we don't need to keep the property that identifies the > > feature. > > For the kernel Image, how do we remove these sections? The objcopy flags > in arch/arm64/boot/Makefile look both insufficient and out of date. My > vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" > segment. > The latter is the fault of the libstub make rules, that prepend .init to all section names. I'm not sure if there is a point to having PAC and/or BTI in the EFI stub, given that it runs under the control of the firmware, with its memory mappings and PAC configuration etc.
On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > On Wed, 24 Jun 2020 at 12:44, Will Deacon <will@kernel.org> wrote: > > On Tue, Jun 23, 2020 at 09:44:11PM -0700, Kees Cook wrote: > > > On Tue, Jun 23, 2020 at 08:31:42PM -0700, 'Fangrui Song' via Clang Built Linux wrote: > > > > arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on > > > > > > > > ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) > > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti > > > > else > > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf > > > > endif > > > > > > > > This option creates .note.gnu.property: > > > > > > > > % readelf -n drivers/firmware/efi/libstub/efi-stub.o > > > > > > > > Displaying notes found in: .note.gnu.property > > > > Owner Data size Description > > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > > Properties: AArch64 feature: PAC > > > > > > > > If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying > > > > -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf > > > > > > We want to keep the branch protection enabled. But since it's not a > > > "regular" ELF, we don't need to keep the property that identifies the > > > feature. > > > > For the kernel Image, how do we remove these sections? The objcopy flags > > in arch/arm64/boot/Makefile look both insufficient and out of date. My > > vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" > > segment. > > > > The latter is the fault of the libstub make rules, that prepend .init > to all section names. Hmm. I tried adding -mbranch-protection=none to arm64 cflags for the stub, but I still see this note in vmlinux. It looks like it comes in via the stub copy of lib-ctype.o, but I don't know why that would force the note. The cflags look ok to me [1] and I confirmed that the note is being generated by the compiler. > I'm not sure if there is a point to having PAC and/or BTI in the EFI > stub, given that it runs under the control of the firmware, with its > memory mappings and PAC configuration etc. Agreed, I just can't figure out how to get rid of the note. Will [1] -mlittle-endian -DKASAN_SHADOW_SCALE_SHIFT=3 -Qunused-arguments -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 --target=aarch64-linux-gnu --prefix=/usr/local/google/home/willdeacon/bin/ --gcc-toolchain=/usr/local/google/home/willdeacon -no-integrated-as -Werror=unknown-warning-option -mgeneral-regs-only -DCONFIG_CC_HAS_K_CONSTRAINT=1 -fno-asynchronous-unwind-tables -mbranch-protection=pac-ret+leaf+bti -Wa,-march=armv8.3-a -DKASAN_SHADOW_SCALE_SHIFT=3 -fno-delete-null-pointer-checks -Wno-address-of-packed-member -O2 -Wframe-larger-than=2048 -fstack-protector-strong -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-const-variable -fno-omit-frame-pointer -fno-optimize-sibling-calls -g -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-merge-all-constants -fno-stack- check -Werror=date-time -Werror=incompatible-pointer-types -fmacro-prefix-map=./= -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-tautological-constant-out-of-range-compare -fpie -mbranch-protection=none -I./scripts/dtc/libfdt -Os -DDISABLE_BRANCH_PROFILING -include ./drivers/firmware/efi/libstub/hidden.h -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fno-addrsig -D__DISABLE_EXPORTS -DKBUILD_MODFILE='"drivers/firmware/efi/libstub/lib-ctype"' -DKBUILD_BASENAME='"lib_ctype"' -DKBUILD_MODNAME='"lib_ctype"' -c -o drivers/firmware/efi/libstub/lib-ctype.o lib/ctype.c
On Wed, Jun 24, 2020 at 12:26:47PM +0100, Will Deacon wrote: > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > On Wed, 24 Jun 2020 at 12:44, Will Deacon <will@kernel.org> wrote: > > > On Tue, Jun 23, 2020 at 09:44:11PM -0700, Kees Cook wrote: > > > > On Tue, Jun 23, 2020 at 08:31:42PM -0700, 'Fangrui Song' via Clang Built Linux wrote: > > > > > arch/arm64/Kconfig enables ARM64_PTR_AUTH by default. When the config is on > > > > > > > > > > ifeq ($(CONFIG_ARM64_BTI_KERNEL),y) > > > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=pac-ret+leaf+bti > > > > > else > > > > > branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf > > > > > endif > > > > > > > > > > This option creates .note.gnu.property: > > > > > > > > > > % readelf -n drivers/firmware/efi/libstub/efi-stub.o > > > > > > > > > > Displaying notes found in: .note.gnu.property > > > > > Owner Data size Description > > > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > > > Properties: AArch64 feature: PAC > > > > > > > > > > If .note.gnu.property is not desired in drivers/firmware/efi/libstub, specifying > > > > > -mbranch-protection=none can override -mbranch-protection=pac-ret+leaf > > > > > > > > We want to keep the branch protection enabled. But since it's not a > > > > "regular" ELF, we don't need to keep the property that identifies the > > > > feature. > > > > > > For the kernel Image, how do we remove these sections? The objcopy flags > > > in arch/arm64/boot/Makefile look both insufficient and out of date. My > > > vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" > > > segment. > > > > > > > The latter is the fault of the libstub make rules, that prepend .init > > to all section names. > > Hmm. I tried adding -mbranch-protection=none to arm64 cflags for the stub, > but I still see this note in vmlinux. It looks like it comes in via the > stub copy of lib-ctype.o, but I don't know why that would force the > note. The cflags look ok to me [1] and I confirmed that the note is > being generated by the compiler. > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > stub, given that it runs under the control of the firmware, with its > > memory mappings and PAC configuration etc. > > Agreed, I just can't figure out how to get rid of the note. Because this section is generated by the linker itself I think you might have to send it to /DISCARD/ in the link, or strip it explicitly after linking. Cheers ---Dave
On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > I'm not sure if there is a point to having PAC and/or BTI in the EFI > stub, given that it runs under the control of the firmware, with its > memory mappings and PAC configuration etc. Is BTI being ignored when the firmware runs?
On Wed, Jun 24, 2020 at 02:48:55PM +0100, Dave Martin wrote: > On Wed, Jun 24, 2020 at 12:26:47PM +0100, Will Deacon wrote: > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > On Wed, 24 Jun 2020 at 12:44, Will Deacon <will@kernel.org> wrote: > > > > For the kernel Image, how do we remove these sections? The objcopy flags > > > > in arch/arm64/boot/Makefile look both insufficient and out of date. My > > > > vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" > > > > segment. > > > > > > The latter is the fault of the libstub make rules, that prepend .init > > > to all section names. > > > > Hmm. I tried adding -mbranch-protection=none to arm64 cflags for the stub, > > but I still see this note in vmlinux. It looks like it comes in via the > > stub copy of lib-ctype.o, but I don't know why that would force the > > note. The cflags look ok to me [1] and I confirmed that the note is > > being generated by the compiler. > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > stub, given that it runs under the control of the firmware, with its > > > memory mappings and PAC configuration etc. > > > > Agreed, I just can't figure out how to get rid of the note. > > Because this section is generated by the linker itself I think you might > have to send it to /DISCARD/ in the link, or strip it explicitly after > linking. Right, but why is the linker generating that section in the first place? I'm compiling with -mbranch-protection=none and all the other objects linked into the stub do not have the section. I wonder if it's because lib/ctype.c doesn't have any executable code... Will
On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > stub, given that it runs under the control of the firmware, with its > > memory mappings and PAC configuration etc. > > Is BTI being ignored when the firmware runs? > Given that it requires the 'guarded' attribute to be set in the page tables, and the fact that the UEFI spec does not require it for executables that it invokes, nor describes any means of annotating such executables as having been built with BTI annotations, I think we can safely assume that the EFI stub will execute with BTI disabled in the foreseeable future.
On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > stub, given that it runs under the control of the firmware, with its > > > memory mappings and PAC configuration etc. > > > > Is BTI being ignored when the firmware runs? > > Given that it requires the 'guarded' attribute to be set in the page > tables, and the fact that the UEFI spec does not require it for > executables that it invokes, nor describes any means of annotating > such executables as having been built with BTI annotations, I think we > can safely assume that the EFI stub will execute with BTI disabled in > the foreseeable future. yaaaaaay. *sigh* How long until EFI catches up? That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable it, we'll be ready?
On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > stub, given that it runs under the control of the firmware, with its > > > > memory mappings and PAC configuration etc. > > > > > > Is BTI being ignored when the firmware runs? > > > > Given that it requires the 'guarded' attribute to be set in the page > > tables, and the fact that the UEFI spec does not require it for > > executables that it invokes, nor describes any means of annotating > > such executables as having been built with BTI annotations, I think we > > can safely assume that the EFI stub will execute with BTI disabled in > > the foreseeable future. > > yaaaaaay. *sigh* How long until EFI catches up? > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > it, we'll be ready? > Sure. Although I anticipate that we'll need to set some flag in the PE/COFF header to enable it, and so any BTI opcodes we emit without that will never take effect in practice.
On Wed, Jun 24, 2020 at 04:26:46PM +0100, Will Deacon wrote: > On Wed, Jun 24, 2020 at 02:48:55PM +0100, Dave Martin wrote: > > On Wed, Jun 24, 2020 at 12:26:47PM +0100, Will Deacon wrote: > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > On Wed, 24 Jun 2020 at 12:44, Will Deacon <will@kernel.org> wrote: > > > > > For the kernel Image, how do we remove these sections? The objcopy flags > > > > > in arch/arm64/boot/Makefile look both insufficient and out of date. My > > > > > vmlinux ends up with both a ".notes" and a ".init.note.gnu.property" > > > > > segment. > > > > > > > > The latter is the fault of the libstub make rules, that prepend .init > > > > to all section names. > > > > > > Hmm. I tried adding -mbranch-protection=none to arm64 cflags for the stub, > > > but I still see this note in vmlinux. It looks like it comes in via the > > > stub copy of lib-ctype.o, but I don't know why that would force the > > > note. The cflags look ok to me [1] and I confirmed that the note is > > > being generated by the compiler. > > > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > stub, given that it runs under the control of the firmware, with its > > > > memory mappings and PAC configuration etc. > > > > > > Agreed, I just can't figure out how to get rid of the note. > > > > Because this section is generated by the linker itself I think you might > > have to send it to /DISCARD/ in the link, or strip it explicitly after > > linking. > > Right, but why is the linker generating that section in the first place? I'm > compiling with -mbranch-protection=none and all the other objects linked > into the stub do not have the section. > > I wonder if it's because lib/ctype.c doesn't have any executable code... What compiler and flags are you using for the affected object? I don't see this with gcc so far. I wonder if this is a hole in the specs: the property could logically be emitted in any codeless object, since turning on BTI will obviously not break that object. For different linkers and compilers to interoperate though, the specs would need to say what to do in that situation. Cheers ---Dave > > Will > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 24, 2020 at 05:48:41PM +0200, Ard Biesheuvel wrote: > On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > > stub, given that it runs under the control of the firmware, with its > > > > > memory mappings and PAC configuration etc. > > > > > > > > Is BTI being ignored when the firmware runs? > > > > > > Given that it requires the 'guarded' attribute to be set in the page > > > tables, and the fact that the UEFI spec does not require it for > > > executables that it invokes, nor describes any means of annotating > > > such executables as having been built with BTI annotations, I think we > > > can safely assume that the EFI stub will execute with BTI disabled in > > > the foreseeable future. > > > > yaaaaaay. *sigh* How long until EFI catches up? > > > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > > it, we'll be ready? > > > > Sure. Although I anticipate that we'll need to set some flag in the > PE/COFF header to enable it, and so any BTI opcodes we emit without > that will never take effect in practice. In the meantime, it is possible to build all the in-tree parts of EFI for BTI, and just turn it off for out-of-tree EFI binaries? If there's no easy way to do this though, I guess we should wait for / push for a PE/COFF flag to describe this properly. Cheers ---Dave
On Wed, 24 Jun 2020 at 18:29, Dave Martin <Dave.Martin@arm.com> wrote: > > On Wed, Jun 24, 2020 at 05:48:41PM +0200, Ard Biesheuvel wrote: > > On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > > > > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > > > stub, given that it runs under the control of the firmware, with its > > > > > > memory mappings and PAC configuration etc. > > > > > > > > > > Is BTI being ignored when the firmware runs? > > > > > > > > Given that it requires the 'guarded' attribute to be set in the page > > > > tables, and the fact that the UEFI spec does not require it for > > > > executables that it invokes, nor describes any means of annotating > > > > such executables as having been built with BTI annotations, I think we > > > > can safely assume that the EFI stub will execute with BTI disabled in > > > > the foreseeable future. > > > > > > yaaaaaay. *sigh* How long until EFI catches up? > > > > > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > > > it, we'll be ready? > > > > > > > Sure. Although I anticipate that we'll need to set some flag in the > > PE/COFF header to enable it, and so any BTI opcodes we emit without > > that will never take effect in practice. > > In the meantime, it is possible to build all the in-tree parts of EFI > for BTI, and just turn it off for out-of-tree EFI binaries? > Not sure I understand the question. What do you mean by out-of-tree EFI binaries? And how would the firmware (which is out of tree itself, and is in charge of the page tables, vector table, timer interrupt etc when the EFI stub executes) distinguish such binaries from the EFI stub? > If there's no easy way to do this though, I guess we should wait for / > push for a PE/COFF flag to describe this properly. > Yeah good point. I will take this to the forum.
On Wed, Jun 24, 2020 at 06:40:48PM +0200, Ard Biesheuvel wrote: > On Wed, 24 Jun 2020 at 18:29, Dave Martin <Dave.Martin@arm.com> wrote: > > > > On Wed, Jun 24, 2020 at 05:48:41PM +0200, Ard Biesheuvel wrote: > > > On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > > > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > > > > stub, given that it runs under the control of the firmware, with its > > > > > > > memory mappings and PAC configuration etc. > > > > > > > > > > > > Is BTI being ignored when the firmware runs? > > > > > > > > > > Given that it requires the 'guarded' attribute to be set in the page > > > > > tables, and the fact that the UEFI spec does not require it for > > > > > executables that it invokes, nor describes any means of annotating > > > > > such executables as having been built with BTI annotations, I think we > > > > > can safely assume that the EFI stub will execute with BTI disabled in > > > > > the foreseeable future. > > > > > > > > yaaaaaay. *sigh* How long until EFI catches up? > > > > > > > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > > > > it, we'll be ready? > > > > > > > > > > Sure. Although I anticipate that we'll need to set some flag in the > > > PE/COFF header to enable it, and so any BTI opcodes we emit without > > > that will never take effect in practice. > > > > In the meantime, it is possible to build all the in-tree parts of EFI > > for BTI, and just turn it off for out-of-tree EFI binaries? > > > > Not sure I understand the question. What do you mean by out-of-tree > EFI binaries? And how would the firmware (which is out of tree itself, > and is in charge of the page tables, vector table, timer interrupt etc > when the EFI stub executes) distinguish such binaries from the EFI > stub? I'm not an EFI expert, but I'm guessing that you configure EFI with certain compiler flags and build it. Possibly some standalone EFI executables are built out of the same tree and shipped with the firmware from the same build, but I'm speculating. If not, we can just run all EFI executables with BTI off. > > If there's no easy way to do this though, I guess we should wait for / > > push for a PE/COFF flag to describe this properly. > > > > Yeah good point. I will take this to the forum. In the interim, we could set the GP bit in EFI's page tables for the executable code from the firmware image if we want this protection, but turn it off in pages mapping the executable code of EFI executables. This is better than nothing. Cheers ---Dave
On Wed, 24 Jun 2020 at 19:16, Dave Martin <Dave.Martin@arm.com> wrote: > > On Wed, Jun 24, 2020 at 06:40:48PM +0200, Ard Biesheuvel wrote: > > On Wed, 24 Jun 2020 at 18:29, Dave Martin <Dave.Martin@arm.com> wrote: > > > > > > On Wed, Jun 24, 2020 at 05:48:41PM +0200, Ard Biesheuvel wrote: > > > > On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > > > > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > > > > > stub, given that it runs under the control of the firmware, with its > > > > > > > > memory mappings and PAC configuration etc. > > > > > > > > > > > > > > Is BTI being ignored when the firmware runs? > > > > > > > > > > > > Given that it requires the 'guarded' attribute to be set in the page > > > > > > tables, and the fact that the UEFI spec does not require it for > > > > > > executables that it invokes, nor describes any means of annotating > > > > > > such executables as having been built with BTI annotations, I think we > > > > > > can safely assume that the EFI stub will execute with BTI disabled in > > > > > > the foreseeable future. > > > > > > > > > > yaaaaaay. *sigh* How long until EFI catches up? > > > > > > > > > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > > > > > it, we'll be ready? > > > > > > > > > > > > > Sure. Although I anticipate that we'll need to set some flag in the > > > > PE/COFF header to enable it, and so any BTI opcodes we emit without > > > > that will never take effect in practice. > > > > > > In the meantime, it is possible to build all the in-tree parts of EFI > > > for BTI, and just turn it off for out-of-tree EFI binaries? > > > > > > > Not sure I understand the question. What do you mean by out-of-tree > > EFI binaries? And how would the firmware (which is out of tree itself, > > and is in charge of the page tables, vector table, timer interrupt etc > > when the EFI stub executes) distinguish such binaries from the EFI > > stub? > > I'm not an EFI expert, but I'm guessing that you configure EFI with > certain compiler flags and build it. 'EFI' is not something you build. It is a specification that describes how a conformant firmware implementation interfaces with a conformant OS. Sorry to be pedantic, but that is really quite relevant. By adhering to the EFI spec rigorously, we no longer have to care about who implements the opposite side, and how. So yes, of course there are ways to build the opposite side with BTI enabled, in a way that all its constituent pieces keep working as expected. A typical EDK2 based implementation of EFI consists of 50-100 individual PE/COFF executables that all get loaded, relocated and started like ordinary user space programs. What we cannot do, though, is invent our own Linux specific way of decorating the kernel's PE/COFF header with an annotation that instructs a Linux specific EFI loader when to enable the GP bit for the .text pages. > Possibly some standalone EFI > executables are built out of the same tree and shipped with the > firmware from the same build, but I'm speculating. If not, we can just > run all EFI executables with BTI off. > > > > If there's no easy way to do this though, I guess we should wait for / > > > push for a PE/COFF flag to describe this properly. > > > > > > > Yeah good point. I will take this to the forum. > > In the interim, we could set the GP bit in EFI's page tables for the > executable code from the firmware image if we want this protection, but > turn it off in pages mapping the executable code of EFI executables. > This is better than nothing. > We need to distinguish between the EFI stub and the EFI runtime services here. The EFI stub consists of kernel code that executes in the context of the firmware, at which point the loader has no control whatsoever over page tables, vector tables, etc. This is the stage where the loading and starting of PE/COFF images takes place. If we want to enable BTI for code running in this context, we need PE/COFF annotations, as discussed above. The EFI runtime services are firmware code that gets invoked by the OS at runtime. Whether or not such code is emitted with BTI annotations is a separate matter (but should also be taken to the forum nonetheless), and does not need any changes at the PE/COFF level. However, for this code, I'd like the sandboxing to be much more rigorous than it is today, to the point where the security it provides doesn't even matter deeply to the OS itself. (I had some patches a while ago that reused the KPTI infrastructure to unmap the entire kernel while EFI runtime services are in progress. There was also an intern in the team that implemented something similar on top of KVM)
On Wed, 24 Jun 2020 at 20:23, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 24 Jun 2020 at 19:16, Dave Martin <Dave.Martin@arm.com> wrote: > > > > On Wed, Jun 24, 2020 at 06:40:48PM +0200, Ard Biesheuvel wrote: > > > On Wed, 24 Jun 2020 at 18:29, Dave Martin <Dave.Martin@arm.com> wrote: > > > > > > > > On Wed, Jun 24, 2020 at 05:48:41PM +0200, Ard Biesheuvel wrote: > > > > > On Wed, 24 Jun 2020 at 17:45, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > > > On Wed, Jun 24, 2020 at 05:31:06PM +0200, Ard Biesheuvel wrote: > > > > > > > On Wed, 24 Jun 2020 at 17:21, Kees Cook <keescook@chromium.org> wrote: > > > > > > > > > > > > > > > > On Wed, Jun 24, 2020 at 12:46:32PM +0200, Ard Biesheuvel wrote: > > > > > > > > > I'm not sure if there is a point to having PAC and/or BTI in the EFI > > > > > > > > > stub, given that it runs under the control of the firmware, with its > > > > > > > > > memory mappings and PAC configuration etc. > > > > > > > > > > > > > > > > Is BTI being ignored when the firmware runs? > > > > > > > > > > > > > > Given that it requires the 'guarded' attribute to be set in the page > > > > > > > tables, and the fact that the UEFI spec does not require it for > > > > > > > executables that it invokes, nor describes any means of annotating > > > > > > > such executables as having been built with BTI annotations, I think we > > > > > > > can safely assume that the EFI stub will execute with BTI disabled in > > > > > > > the foreseeable future. > > > > > > > > > > > > yaaaaaay. *sigh* How long until EFI catches up? > > > > > > > > > > > > That said, BTI shouldn't _hurt_, right? If EFI ever decides to enable > > > > > > it, we'll be ready? > > > > > > > > > > > > > > > > Sure. Although I anticipate that we'll need to set some flag in the > > > > > PE/COFF header to enable it, and so any BTI opcodes we emit without > > > > > that will never take effect in practice. > > > > > > > > In the meantime, it is possible to build all the in-tree parts of EFI > > > > for BTI, and just turn it off for out-of-tree EFI binaries? > > > > > > > > > > Not sure I understand the question. What do you mean by out-of-tree > > > EFI binaries? And how would the firmware (which is out of tree itself, > > > and is in charge of the page tables, vector table, timer interrupt etc > > > when the EFI stub executes) distinguish such binaries from the EFI > > > stub? > > > > I'm not an EFI expert, but I'm guessing that you configure EFI with > > certain compiler flags and build it. > > 'EFI' is not something you build. It is a specification that describes > how a conformant firmware implementation interfaces with a conformant > OS. > > Sorry to be pedantic, but that is really quite relevant. By adhering > to the EFI spec rigorously, we no longer have to care about who > implements the opposite side, and how. > > So yes, of course there are ways to build the opposite side with BTI > enabled, in a way that all its constituent pieces keep working as > expected. A typical EDK2 based implementation of EFI consists of > 50-100 individual PE/COFF executables that all get loaded, relocated > and started like ordinary user space programs. > > What we cannot do, though, is invent our own Linux specific way of > decorating the kernel's PE/COFF header with an annotation that > instructs a Linux specific EFI loader when to enable the GP bit for > the .text pages. > > > Possibly some standalone EFI > > executables are built out of the same tree and shipped with the > > firmware from the same build, but I'm speculating. If not, we can just > > run all EFI executables with BTI off. > > > > > > If there's no easy way to do this though, I guess we should wait for / > > > > push for a PE/COFF flag to describe this properly. > > > > > > > > > > Yeah good point. I will take this to the forum. > > > > In the interim, we could set the GP bit in EFI's page tables for the > > executable code from the firmware image if we want this protection, but > > turn it off in pages mapping the executable code of EFI executables. > > This is better than nothing. > > > > We need to distinguish between the EFI stub and the EFI runtime services here. > > The EFI stub consists of kernel code that executes in the context of > the firmware, at which point the loader has no control whatsoever over > page tables, vector tables, etc. This is the stage where the loading > and starting of PE/COFF images takes place. If we want to enable BTI > for code running in this context, we need PE/COFF annotations, as > discussed above. > > The EFI runtime services are firmware code that gets invoked by the OS > at runtime. Whether or not such code is emitted with BTI annotations > is a separate matter (but should also be taken to the forum > nonetheless), and does not need any changes at the PE/COFF level. > However, for this code, I'd like the sandboxing to be much more > rigorous than it is today, to the point where the security it provides ... the security *bti* provides ... > doesn't even matter deeply to the OS itself. (I had some patches a > while ago that reused the KPTI infrastructure to unmap the entire > kernel while EFI runtime services are in progress. There was also an > intern in the team that implemented something similar on top of KVM)
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 75daaf20374e..9d2d2e784bca 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -66,6 +66,9 @@ lib-$(CONFIG_X86) += x86-stub.o CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) +# Remove unwanted sections first. +STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property + # # For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the # .bss section, so the .bss section of the EFI stub needs to be included in the
In preparation for adding --orphan-handling=warn to more architectures, make sure unwanted sections don't end up appearing under the .init section prefix that libstub adds to itself during objcopy. Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/firmware/efi/libstub/Makefile | 3 +++ 1 file changed, 3 insertions(+)