| Message ID | 1444330924-17830-4-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | e8f3010f7326c00368dbc057bd052bec80dfc072 |
| Headers | show |
2015-10-08 22:02 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > --- a/arch/arm64/kernel/image.h > +++ b/arch/arm64/kernel/image.h > @@ -59,4 +59,31 @@ > _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ > _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); > > +#ifdef CONFIG_EFI > + > +/* > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to > + * isolate it from the kernel proper. The following symbols are legally > + * accessed by the stub, so provide some aliases to make them accessible. > + * Only include data symbols here, or text symbols of functions that are > + * guaranteed to be safe when executed at another offset than they were > + * linked at. The routines below are all implemented in assembler in a > + * position independent manner > + */ > +__efistub_memcmp = __pi_memcmp; > +__efistub_memchr = __pi_memchr; > +__efistub_memcpy = __pi_memcpy; > +__efistub_memmove = __pi_memmove; > +__efistub_memset = __pi_memset; > +__efistub_strlen = __pi_strlen; > +__efistub_strcmp = __pi_strcmp; > +__efistub_strncmp = __pi_strncmp; > +__efistub___flush_dcache_area = __pi___flush_dcache_area; So why we need these __pi_* aliases? We could just do __efistub_memcmp = memcmp; Right? > + > +__efistub__text = _text; > +__efistub__end = _end; > +__efistub__edata = _edata; > + > +#endif
On Fri, Oct 09, 2015 at 11:12:24AM +0300, Andrey Ryabinin wrote: > 2015-10-08 22:02 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > > --- a/arch/arm64/kernel/image.h > > +++ b/arch/arm64/kernel/image.h > > @@ -59,4 +59,31 @@ > > _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ > > _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); > > > > +#ifdef CONFIG_EFI > > + > > +/* > > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to > > + * isolate it from the kernel proper. The following symbols are legally > > + * accessed by the stub, so provide some aliases to make them accessible. > > + * Only include data symbols here, or text symbols of functions that are > > + * guaranteed to be safe when executed at another offset than they were > > + * linked at. The routines below are all implemented in assembler in a > > + * position independent manner > > + */ > > +__efistub_memcmp = __pi_memcmp; > > +__efistub_memchr = __pi_memchr; > > +__efistub_memcpy = __pi_memcpy; > > +__efistub_memmove = __pi_memmove; > > +__efistub_memset = __pi_memset; > > +__efistub_strlen = __pi_strlen; > > +__efistub_strcmp = __pi_strcmp; > > +__efistub_strncmp = __pi_strncmp; > > +__efistub___flush_dcache_area = __pi___flush_dcache_area; > > So why we need these __pi_* aliases? > We could just do __efistub_memcmp = memcmp; Right? We *could*, but that defeats the whole purpose of tagging position-independent functions explicitly in the kernel text. Will
2015-10-09 12:10 GMT+03:00 Will Deacon <will.deacon@arm.com>: > On Fri, Oct 09, 2015 at 11:12:24AM +0300, Andrey Ryabinin wrote: >> 2015-10-08 22:02 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: >> > --- a/arch/arm64/kernel/image.h >> > +++ b/arch/arm64/kernel/image.h >> > @@ -59,4 +59,31 @@ >> > _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ >> > _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); >> > >> > +#ifdef CONFIG_EFI >> > + >> > +/* >> > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to >> > + * isolate it from the kernel proper. The following symbols are legally >> > + * accessed by the stub, so provide some aliases to make them accessible. >> > + * Only include data symbols here, or text symbols of functions that are >> > + * guaranteed to be safe when executed at another offset than they were >> > + * linked at. The routines below are all implemented in assembler in a >> > + * position independent manner >> > + */ >> > +__efistub_memcmp = __pi_memcmp; >> > +__efistub_memchr = __pi_memchr; >> > +__efistub_memcpy = __pi_memcpy; >> > +__efistub_memmove = __pi_memmove; >> > +__efistub_memset = __pi_memset; >> > +__efistub_strlen = __pi_strlen; >> > +__efistub_strcmp = __pi_strcmp; >> > +__efistub_strncmp = __pi_strncmp; >> > +__efistub___flush_dcache_area = __pi___flush_dcache_area; >> >> So why we need these __pi_* aliases? >> We could just do __efistub_memcmp = memcmp; Right? > > We *could*, but that defeats the whole purpose of tagging > position-independent functions explicitly in the kernel text. > I just don't get that "the whole purpose of tagging". Yes, the EFI stub is allowed to call only PI kernel functions. But the EFI stub already protected by __efistub_ namespace. If we want to use some new PI function in the stub, we would need add it into this list at first. So that __pi_ namespace doesn't bring any protection or isolation.
On Fri, Oct 09, 2015 at 12:40:21PM +0300, Andrey Ryabinin wrote: > 2015-10-09 12:10 GMT+03:00 Will Deacon <will.deacon@arm.com>: > > On Fri, Oct 09, 2015 at 11:12:24AM +0300, Andrey Ryabinin wrote: > >> 2015-10-08 22:02 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > >> > --- a/arch/arm64/kernel/image.h > >> > +++ b/arch/arm64/kernel/image.h > >> > @@ -59,4 +59,31 @@ > >> > _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ > >> > _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); > >> > > >> > +#ifdef CONFIG_EFI > >> > + > >> > +/* > >> > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to > >> > + * isolate it from the kernel proper. The following symbols are legally > >> > + * accessed by the stub, so provide some aliases to make them accessible. > >> > + * Only include data symbols here, or text symbols of functions that are > >> > + * guaranteed to be safe when executed at another offset than they were > >> > + * linked at. The routines below are all implemented in assembler in a > >> > + * position independent manner > >> > + */ > >> > +__efistub_memcmp = __pi_memcmp; > >> > +__efistub_memchr = __pi_memchr; > >> > +__efistub_memcpy = __pi_memcpy; > >> > +__efistub_memmove = __pi_memmove; > >> > +__efistub_memset = __pi_memset; > >> > +__efistub_strlen = __pi_strlen; > >> > +__efistub_strcmp = __pi_strcmp; > >> > +__efistub_strncmp = __pi_strncmp; > >> > +__efistub___flush_dcache_area = __pi___flush_dcache_area; > >> > >> So why we need these __pi_* aliases? > >> We could just do __efistub_memcmp = memcmp; Right? > > > > We *could*, but that defeats the whole purpose of tagging > > position-independent functions explicitly in the kernel text. > > > > I just don't get that "the whole purpose of tagging". > > Yes, the EFI stub is allowed to call only PI kernel functions. > But the EFI stub already protected by __efistub_ namespace. > If we want to use some new PI function in the stub, we would need add > it into this list at first. > So that __pi_ namespace doesn't bring any protection or isolation. What it does it force people making future changes to the __pi_* functions to think about the stub, otherwise if the function happens to be position-independent when efistub starts using it, it could easily break due to some future patch where the author didn't realise that it was being used in that manner. Will
2015-10-09 12:43 GMT+03:00 Will Deacon <will.deacon@arm.com>: > On Fri, Oct 09, 2015 at 12:40:21PM +0300, Andrey Ryabinin wrote: >> 2015-10-09 12:10 GMT+03:00 Will Deacon <will.deacon@arm.com>: >> > On Fri, Oct 09, 2015 at 11:12:24AM +0300, Andrey Ryabinin wrote: >> >> 2015-10-08 22:02 GMT+03:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: >> >> > --- a/arch/arm64/kernel/image.h >> >> > +++ b/arch/arm64/kernel/image.h >> >> > @@ -59,4 +59,31 @@ >> >> > _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ >> >> > _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); >> >> > >> >> > +#ifdef CONFIG_EFI >> >> > + >> >> > +/* >> >> > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to >> >> > + * isolate it from the kernel proper. The following symbols are legally >> >> > + * accessed by the stub, so provide some aliases to make them accessible. >> >> > + * Only include data symbols here, or text symbols of functions that are >> >> > + * guaranteed to be safe when executed at another offset than they were >> >> > + * linked at. The routines below are all implemented in assembler in a >> >> > + * position independent manner >> >> > + */ >> >> > +__efistub_memcmp = __pi_memcmp; >> >> > +__efistub_memchr = __pi_memchr; >> >> > +__efistub_memcpy = __pi_memcpy; >> >> > +__efistub_memmove = __pi_memmove; >> >> > +__efistub_memset = __pi_memset; >> >> > +__efistub_strlen = __pi_strlen; >> >> > +__efistub_strcmp = __pi_strcmp; >> >> > +__efistub_strncmp = __pi_strncmp; >> >> > +__efistub___flush_dcache_area = __pi___flush_dcache_area; >> >> >> >> So why we need these __pi_* aliases? >> >> We could just do __efistub_memcmp = memcmp; Right? >> > >> > We *could*, but that defeats the whole purpose of tagging >> > position-independent functions explicitly in the kernel text. >> > >> >> I just don't get that "the whole purpose of tagging". >> >> Yes, the EFI stub is allowed to call only PI kernel functions. >> But the EFI stub already protected by __efistub_ namespace. >> If we want to use some new PI function in the stub, we would need add >> it into this list at first. >> So that __pi_ namespace doesn't bring any protection or isolation. > > What it does it force people making future changes to the __pi_* functions > to think about the stub, otherwise if the function happens to be > position-independent when efistub starts using it, it could easily break > due to some future patch where the author didn't realise that it was > being used in that manner. > That makes sense, thanks. > Will
On Thu, 08 Oct, at 08:02:04PM, Ard Biesheuvel wrote: > Since arm64 does not use a builtin decompressor, the EFI stub is built > into the kernel proper. So far, this has been working fine, but actually, > since the stub is in fact a PE/COFF relocatable binary that is executed > at an unknown offset in the 1:1 mapping provided by the UEFI firmware, we > should not be seamlessly sharing code with the kernel proper, which is a > position dependent executable linked at a high virtual offset. > > So instead, separate the contents of libstub and its dependencies, by > putting them into their own namespace by prefixing all of its symbols > with __efistub. This way, we have tight control over what parts of the > kernel proper are referenced by the stub. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm64/kernel/Makefile | 12 ++++- > arch/arm64/kernel/efi-entry.S | 10 ++-- > arch/arm64/kernel/head.S | 14 ++--- > arch/arm64/kernel/image.h | 27 ++++++++++ > drivers/firmware/efi/libstub/Makefile | 39 +++++++++++--- > drivers/firmware/efi/libstub/string.c | 57 ++++++++++++++++++++ > 6 files changed, 139 insertions(+), 20 deletions(-) Reviewed-by: Matt Fleming <matt.fleming@intel.com>
Ard, On 08.10.15 20:02:04, Ard Biesheuvel wrote: > +quiet_cmd_stubcopy = STUBCPY $@ > + cmd_stubcopy = if $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; then \ > + $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y) \ > + && (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \ > + rm -f $@; /bin/false); else /bin/false; fi This one is causing some more build errors for my setup. Any hint how to fix that? Thanks, -Robert STUBCPY drivers/firmware/efi/libstub/arm-stub.stub.o 0000000000000006 R_AARCH64_ABS32 .debug_abbrev 000000000000000c R_AARCH64_ABS32 .debug_str+0x0000000000000890 0000000000000011 R_AARCH64_ABS32 .debug_str+0x0000000000004ecd 0000000000000015 R_AARCH64_ABS32 .debug_str+0x0000000000000bb1 0000000000000019 R_AARCH64_ABS64 .init.text 0000000000000021 R_AARCH64_ABS64 .init.text+0x00000000000008cc 0000000000000029 R_AARCH64_ABS32 .debug_line 0000000000000030 R_AARCH64_ABS32 .debug_str+0x00000000000000eb 0000000000000047 R_AARCH64_ABS32 .debug_str+0x0000000000003ca8 0000000000000059 R_AARCH64_ABS32 .debug_str+0x0000000000004798 0000000000000060 R_AARCH64_ABS32 .debug_str+0x0000000000005008 ... 000000000000a4bb R_AARCH64_ABS32 .debug_str+0x0000000000004a75 000000000000a4da R_AARCH64_ABS32 .debug_str+0x00000000000029ee 000000000000a4f7 R_AARCH64_ABS32 .debug_str+0x0000000000001d95 000000000000a519 R_AARCH64_ABS32 .debug_str+0x0000000000000773 0000000000000006 R_AARCH64_ABS32 .debug_info 0000000000000010 R_AARCH64_ABS64 .init.text 0000000000000623 R_AARCH64_ABS64 .init.text 0000000000000014 R_AARCH64_ABS32 .debug_frame 0000000000000018 R_AARCH64_ABS64 .init.text 000000000000002c R_AARCH64_ABS32 .debug_frame 0000000000000030 R_AARCH64_ABS64 .init.text+0x0000000000000018 0000000000000074 R_AARCH64_ABS32 .debug_frame 0000000000000078 R_AARCH64_ABS64 .init.text+0x0000000000000140 000000000000009c R_AARCH64_ABS32 .debug_frame 00000000000000a0 R_AARCH64_ABS64 .init.text+0x0000000000000158 00000000000000c4 R_AARCH64_ABS32 .debug_frame 00000000000000c8 R_AARCH64_ABS64 .init.text+0x0000000000000170 000000000000012c R_AARCH64_ABS32 .debug_frame 0000000000000130 R_AARCH64_ABS64 .init.text+0x0000000000000390 0000000000000154 R_AARCH64_ABS32 .debug_frame 0000000000000158 R_AARCH64_ABS64 .init.text+0x00000000000003b0 00000000000001a4 R_AARCH64_ABS32 .debug_frame 00000000000001a8 R_AARCH64_ABS64 .init.text+0x0000000000000780 drivers/firmware/efi/libstub/arm-stub.stub.o: absolute symbol references not allowed in the EFI stub _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 24 November 2015 at 10:34, Robert Richter <robert.richter@caviumnetworks.com> wrote: > Ard, > > On 08.10.15 20:02:04, Ard Biesheuvel wrote: >> +quiet_cmd_stubcopy = STUBCPY $@ >> + cmd_stubcopy = if $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; then \ >> + $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y) \ >> + && (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \ >> + rm -f $@; /bin/false); else /bin/false; fi > > This one is causing some more build errors for my setup. Any hint how > to fix that? > Did you remove -R .debug* from the STUBCOPY_FLAGS-$(CONFIG_ARM64) line? The problem here is that a) .debug contains absolute relocations, which is harmless by itself since they are not used by the program itself, but b) the .debug sections also contain references to the ksymtab and kcrctab sections, which should be removed, and removing those and keeping .debug* sadly doesn't work. -- Ard. > > STUBCPY drivers/firmware/efi/libstub/arm-stub.stub.o > 0000000000000006 R_AARCH64_ABS32 .debug_abbrev > 000000000000000c R_AARCH64_ABS32 .debug_str+0x0000000000000890 > 0000000000000011 R_AARCH64_ABS32 .debug_str+0x0000000000004ecd > 0000000000000015 R_AARCH64_ABS32 .debug_str+0x0000000000000bb1 > 0000000000000019 R_AARCH64_ABS64 .init.text > 0000000000000021 R_AARCH64_ABS64 .init.text+0x00000000000008cc > 0000000000000029 R_AARCH64_ABS32 .debug_line > 0000000000000030 R_AARCH64_ABS32 .debug_str+0x00000000000000eb > 0000000000000047 R_AARCH64_ABS32 .debug_str+0x0000000000003ca8 > 0000000000000059 R_AARCH64_ABS32 .debug_str+0x0000000000004798 > 0000000000000060 R_AARCH64_ABS32 .debug_str+0x0000000000005008 > ... > 000000000000a4bb R_AARCH64_ABS32 .debug_str+0x0000000000004a75 > 000000000000a4da R_AARCH64_ABS32 .debug_str+0x00000000000029ee > 000000000000a4f7 R_AARCH64_ABS32 .debug_str+0x0000000000001d95 > 000000000000a519 R_AARCH64_ABS32 .debug_str+0x0000000000000773 > 0000000000000006 R_AARCH64_ABS32 .debug_info > 0000000000000010 R_AARCH64_ABS64 .init.text > 0000000000000623 R_AARCH64_ABS64 .init.text > 0000000000000014 R_AARCH64_ABS32 .debug_frame > 0000000000000018 R_AARCH64_ABS64 .init.text > 000000000000002c R_AARCH64_ABS32 .debug_frame > 0000000000000030 R_AARCH64_ABS64 .init.text+0x0000000000000018 > 0000000000000074 R_AARCH64_ABS32 .debug_frame > 0000000000000078 R_AARCH64_ABS64 .init.text+0x0000000000000140 > 000000000000009c R_AARCH64_ABS32 .debug_frame > 00000000000000a0 R_AARCH64_ABS64 .init.text+0x0000000000000158 > 00000000000000c4 R_AARCH64_ABS32 .debug_frame > 00000000000000c8 R_AARCH64_ABS64 .init.text+0x0000000000000170 > 000000000000012c R_AARCH64_ABS32 .debug_frame > 0000000000000130 R_AARCH64_ABS64 .init.text+0x0000000000000390 > 0000000000000154 R_AARCH64_ABS32 .debug_frame > 0000000000000158 R_AARCH64_ABS64 .init.text+0x00000000000003b0 > 00000000000001a4 R_AARCH64_ABS32 .debug_frame > 00000000000001a8 R_AARCH64_ABS64 .init.text+0x0000000000000780 > drivers/firmware/efi/libstub/arm-stub.stub.o: absolute symbol references not allowed in the EFI stub _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 22dc9bc781be..7b17f6245f1e 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -20,6 +20,14 @@ arm64-obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ cpufeature.o alternative.o cacheinfo.o \ smp.o smp_spin_table.o topology.o +stub-obj := efi-stub.o efi-entry.o +extra-y := $(stub-obj) +stub-obj := $(patsubst %.o,%.stub.o,$(stub-obj)) + +OBJCOPYFLAGS := --prefix-symbols=__efistub_ +$(obj)/%.stub.o: $(obj)/%.o FORCE + $(call if_changed,objcopy) + arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ sys_compat.o entry32.o \ ../../arm/kernel/opcodes.o @@ -32,7 +40,7 @@ arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o arm64-obj-$(CONFIG_KGDB) += kgdb.o -arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o +arm64-obj-$(CONFIG_EFI) += efi.o $(stub-obj) arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o arm64-obj-$(CONFIG_ACPI) += acpi.o @@ -40,7 +48,7 @@ arm64-obj-$(CONFIG_ACPI) += acpi.o obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) head-y := head.o -extra-y := $(head-y) vmlinux.lds +extra-y += $(head-y) vmlinux.lds # vDSO - this must be built first to generate the symbol offsets $(call objectify,$(arm64-obj-y)): $(obj)/vdso/vdso-offsets.h diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S index 8ce9b0577442..a773db92908b 100644 --- a/arch/arm64/kernel/efi-entry.S +++ b/arch/arm64/kernel/efi-entry.S @@ -29,7 +29,7 @@ * we want to be. The kernel image wants to be placed at TEXT_OFFSET * from start of RAM. */ -ENTRY(efi_stub_entry) +ENTRY(entry) /* * Create a stack frame to save FP/LR with extra space * for image_addr variable passed to efi_entry(). @@ -86,8 +86,8 @@ ENTRY(efi_stub_entry) * entries for the VA range of the current image, so no maintenance is * necessary. */ - adr x0, efi_stub_entry - adr x1, efi_stub_entry_end + adr x0, entry + adr x1, entry_end sub x1, x1, x0 bl __flush_dcache_area @@ -120,5 +120,5 @@ efi_load_fail: ldp x29, x30, [sp], #32 ret -efi_stub_entry_end: -ENDPROC(efi_stub_entry) +entry_end: +ENDPROC(entry) diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 90d09eddd5b2..28a81e948df9 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -120,8 +120,8 @@ efi_head: #endif #ifdef CONFIG_EFI - .globl stext_offset - .set stext_offset, stext - efi_head + .globl __efistub_stext_offset + .set __efistub_stext_offset, stext - efi_head .align 3 pe_header: .ascii "PE" @@ -144,8 +144,8 @@ optional_header: .long _end - stext // SizeOfCode .long 0 // SizeOfInitializedData .long 0 // SizeOfUninitializedData - .long efi_stub_entry - efi_head // AddressOfEntryPoint - .long stext_offset // BaseOfCode + .long __efistub_entry - efi_head // AddressOfEntryPoint + .long __efistub_stext_offset // BaseOfCode extra_header_fields: .quad 0 // ImageBase @@ -162,7 +162,7 @@ extra_header_fields: .long _end - efi_head // SizeOfImage // Everything before the kernel image is considered part of the header - .long stext_offset // SizeOfHeaders + .long __efistub_stext_offset // SizeOfHeaders .long 0 // CheckSum .short 0xa // Subsystem (EFI application) .short 0 // DllCharacteristics @@ -207,9 +207,9 @@ section_table: .byte 0 .byte 0 // end of 0 padding of section name .long _end - stext // VirtualSize - .long stext_offset // VirtualAddress + .long __efistub_stext_offset // VirtualAddress .long _edata - stext // SizeOfRawData - .long stext_offset // PointerToRawData + .long __efistub_stext_offset // PointerToRawData .long 0 // PointerToRelocations (0 for executables) .long 0 // PointerToLineNumbers (0 for executables) diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h index 8fae0756e175..e083af0dd546 100644 --- a/arch/arm64/kernel/image.h +++ b/arch/arm64/kernel/image.h @@ -59,4 +59,31 @@ _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); +#ifdef CONFIG_EFI + +/* + * The EFI stub has its own symbol namespace prefixed by __efistub_, to + * isolate it from the kernel proper. The following symbols are legally + * accessed by the stub, so provide some aliases to make them accessible. + * Only include data symbols here, or text symbols of functions that are + * guaranteed to be safe when executed at another offset than they were + * linked at. The routines below are all implemented in assembler in a + * position independent manner + */ +__efistub_memcmp = __pi_memcmp; +__efistub_memchr = __pi_memchr; +__efistub_memcpy = __pi_memcpy; +__efistub_memmove = __pi_memmove; +__efistub_memset = __pi_memset; +__efistub_strlen = __pi_strlen; +__efistub_strcmp = __pi_strcmp; +__efistub_strncmp = __pi_strncmp; +__efistub___flush_dcache_area = __pi___flush_dcache_area; + +__efistub__text = _text; +__efistub__end = _end; +__efistub__edata = _edata; + +#endif + #endif /* __ASM_IMAGE_H */ diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 816dbe9f4b82..bca9a76cbd33 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -14,6 +14,8 @@ cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic -mno-single-pic-base +cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt + KBUILD_CFLAGS := $(cflags-y) \ $(call cc-option,-ffreestanding) \ $(call cc-option,-fno-stack-protector) @@ -22,7 +24,15 @@ GCOV_PROFILE := n KASAN_SANITIZE := n lib-y := efi-stub-helper.o -lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o + +# include the stub's generic dependencies from lib/ when building for ARM/arm64 +arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c + +$(obj)/lib-%.o: $(srctree)/lib/%.c FORCE + $(call if_changed_rule,cc_o_c) + +lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o \ + $(patsubst %.c,lib-%.o,$(arm-deps)) # # arm64 puts the stub in the kernel proper, which will unnecessarily retain all @@ -30,10 +40,27 @@ lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o # So let's apply the __init annotations at the section level, by prefixing # the section names directly. This will ensure that even all the inline string # literals are covered. +# The fact that the stub and the kernel proper are essentially the same binary +# also means that we need to be extra careful to make sure that the stub does +# not rely on any absolute symbol references, considering that the virtual +# kernel mapping that the linker uses is not active yet when the stub is +# executing. So build all C dependencies of the EFI stub into libstub, and do +# a verification pass to see if any absolute relocations exist in any of the +# object files. # -extra-$(CONFIG_ARM64) := $(lib-y) -lib-$(CONFIG_ARM64) := $(patsubst %.o,%.init.o,$(lib-y)) +extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y) +lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y)) + +STUBCOPY_FLAGS-y := -R .debug* -R *ksymtab* +STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ + --prefix-symbols=__efistub_ +STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS + +$(obj)/%.stub.o: $(obj)/%.o FORCE + $(call if_changed,stubcopy) -OBJCOPYFLAGS := --prefix-alloc-sections=.init -$(obj)/%.init.o: $(obj)/%.o FORCE - $(call if_changed,objcopy) +quiet_cmd_stubcopy = STUBCPY $@ + cmd_stubcopy = if $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; then \ + $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y) \ + && (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \ + rm -f $@; /bin/false); else /bin/false; fi diff --git a/drivers/firmware/efi/libstub/string.c b/drivers/firmware/efi/libstub/string.c new file mode 100644 index 000000000000..09d5a0894343 --- /dev/null +++ b/drivers/firmware/efi/libstub/string.c @@ -0,0 +1,57 @@ +/* + * Taken from: + * linux/lib/string.c + * + * Copyright (C) 1991, 1992 Linus Torvalds + */ + +#include <linux/types.h> +#include <linux/string.h> + +#ifndef __HAVE_ARCH_STRSTR +/** + * strstr - Find the first substring in a %NUL terminated string + * @s1: The string to be searched + * @s2: The string to search for + */ +char *strstr(const char *s1, const char *s2) +{ + size_t l1, l2; + + l2 = strlen(s2); + if (!l2) + return (char *)s1; + l1 = strlen(s1); + while (l1 >= l2) { + l1--; + if (!memcmp(s1, s2, l2)) + return (char *)s1; + s1++; + } + return NULL; +} +#endif + +#ifndef __HAVE_ARCH_STRNCMP +/** + * strncmp - Compare two length-limited strings + * @cs: One string + * @ct: Another string + * @count: The maximum number of bytes to compare + */ +int strncmp(const char *cs, const char *ct, size_t count) +{ + unsigned char c1, c2; + + while (count) { + c1 = *cs++; + c2 = *ct++; + if (c1 != c2) + return c1 < c2 ? -1 : 1; + if (!c1) + break; + count--; + } + return 0; +} +#endif
Since arm64 does not use a builtin decompressor, the EFI stub is built into the kernel proper. So far, this has been working fine, but actually, since the stub is in fact a PE/COFF relocatable binary that is executed at an unknown offset in the 1:1 mapping provided by the UEFI firmware, we should not be seamlessly sharing code with the kernel proper, which is a position dependent executable linked at a high virtual offset. So instead, separate the contents of libstub and its dependencies, by putting them into their own namespace by prefixing all of its symbols with __efistub. This way, we have tight control over what parts of the kernel proper are referenced by the stub. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm64/kernel/Makefile | 12 ++++- arch/arm64/kernel/efi-entry.S | 10 ++-- arch/arm64/kernel/head.S | 14 ++--- arch/arm64/kernel/image.h | 27 ++++++++++ drivers/firmware/efi/libstub/Makefile | 39 +++++++++++--- drivers/firmware/efi/libstub/string.c | 57 ++++++++++++++++++++ 6 files changed, 139 insertions(+), 20 deletions(-)