| Message ID | 20200611124330.252163-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | dd4bc60765873445893037ae73a5f75398a8cd19 |
| Headers | show |
| Series | [v2] arm64: warn on incorrect placement of the kernel by the bootloader | expand |
On Thu, 11 Jun 2020 14:43:30 +0200, Ard Biesheuvel wrote: > Commit cfa7ede20f133c ("arm64: set TEXT_OFFSET to 0x0 in preparation for > removing it entirely") results in boot failures when booting kernels that > are built without KASLR support on broken bootloaders that ignore the > TEXT_OFFSET value passed via the header, and use the default of 0x80000 > instead. > > To work around this, turn CONFIG_RELOCATABLE on by default, even if KASLR > itself (CONFIG_RANDOMIZE_BASE) is turned off, and require CONFIG_EXPERT > to be enabled to deviate from this. Then, emit a warning into the kernel > log if we are not booting via the EFI stub (which is permitted to deviate > from the placement restrictions) and the kernel base address is not placed > according to the rules as laid out in Documentation/arm64/booting.rst. Applied to arm64 (for-next/core), thanks! [1/1] arm64: warn on incorrect placement of the kernel by the bootloader https://git.kernel.org/arm64/c/dd4bc6076587 Cheers,
On Thu, 11 Jun 2020 at 15:23, Will Deacon <will@kernel.org> wrote: > > On Thu, 11 Jun 2020 14:43:30 +0200, Ard Biesheuvel wrote: > > Commit cfa7ede20f133c ("arm64: set TEXT_OFFSET to 0x0 in preparation for > > removing it entirely") results in boot failures when booting kernels that > > are built without KASLR support on broken bootloaders that ignore the > > TEXT_OFFSET value passed via the header, and use the default of 0x80000 > > instead. > > > > To work around this, turn CONFIG_RELOCATABLE on by default, even if KASLR > > itself (CONFIG_RANDOMIZE_BASE) is turned off, and require CONFIG_EXPERT > > to be enabled to deviate from this. Then, emit a warning into the kernel > > log if we are not booting via the EFI stub (which is permitted to deviate > > from the placement restrictions) and the kernel base address is not placed > > according to the rules as laid out in Documentation/arm64/booting.rst. > > Applied to arm64 (for-next/core), thanks! > > [1/1] arm64: warn on incorrect placement of the kernel by the bootloader > https://git.kernel.org/arm64/c/dd4bc6076587 > This test ((u64)_text % MIN_KIMG_ALIGN) != 0 should have been written as ((u64)_text % MIN_KIMG_ALIGN) != TEXT_OFFSET because now, we are throwing the warning when CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET is enabled :-( I was reluctant to add another Makefile rule to add a -DTEXT_OFFSET=... GCC option when compiling this source file, but it appears we need it after all.
On Sat, Jun 13, 2020 at 10:45:10AM +0200, Ard Biesheuvel wrote: > On Thu, 11 Jun 2020 at 15:23, Will Deacon <will@kernel.org> wrote: > > > > On Thu, 11 Jun 2020 14:43:30 +0200, Ard Biesheuvel wrote: > > > Commit cfa7ede20f133c ("arm64: set TEXT_OFFSET to 0x0 in preparation for > > > removing it entirely") results in boot failures when booting kernels that > > > are built without KASLR support on broken bootloaders that ignore the > > > TEXT_OFFSET value passed via the header, and use the default of 0x80000 > > > instead. > > > > > > To work around this, turn CONFIG_RELOCATABLE on by default, even if KASLR > > > itself (CONFIG_RANDOMIZE_BASE) is turned off, and require CONFIG_EXPERT > > > to be enabled to deviate from this. Then, emit a warning into the kernel > > > log if we are not booting via the EFI stub (which is permitted to deviate > > > from the placement restrictions) and the kernel base address is not placed > > > according to the rules as laid out in Documentation/arm64/booting.rst. > > > > Applied to arm64 (for-next/core), thanks! > > > > [1/1] arm64: warn on incorrect placement of the kernel by the bootloader > > https://git.kernel.org/arm64/c/dd4bc6076587 > > > > This test > > ((u64)_text % MIN_KIMG_ALIGN) != 0 > > should have been written as > > ((u64)_text % MIN_KIMG_ALIGN) != TEXT_OFFSET > > because now, we are throwing the warning when > CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET is enabled :-( > > I was reluctant to add another Makefile rule to add a > -DTEXT_OFFSET=... GCC option when compiling this source file, but it > appears we need it after all. FWIW, I'd be in favour of removing CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET at this stage as I don't really see the point in it, especially now that TEXT_OFFSET is on the way out and we've practically forced the use of a relocatable image. What do you think? Will
On Mon, 15 Jun 2020 at 11:51, Will Deacon <will@kernel.org> wrote: > > On Sat, Jun 13, 2020 at 10:45:10AM +0200, Ard Biesheuvel wrote: > > On Thu, 11 Jun 2020 at 15:23, Will Deacon <will@kernel.org> wrote: > > > > > > On Thu, 11 Jun 2020 14:43:30 +0200, Ard Biesheuvel wrote: > > > > Commit cfa7ede20f133c ("arm64: set TEXT_OFFSET to 0x0 in preparation for > > > > removing it entirely") results in boot failures when booting kernels that > > > > are built without KASLR support on broken bootloaders that ignore the > > > > TEXT_OFFSET value passed via the header, and use the default of 0x80000 > > > > instead. > > > > > > > > To work around this, turn CONFIG_RELOCATABLE on by default, even if KASLR > > > > itself (CONFIG_RANDOMIZE_BASE) is turned off, and require CONFIG_EXPERT > > > > to be enabled to deviate from this. Then, emit a warning into the kernel > > > > log if we are not booting via the EFI stub (which is permitted to deviate > > > > from the placement restrictions) and the kernel base address is not placed > > > > according to the rules as laid out in Documentation/arm64/booting.rst. > > > > > > Applied to arm64 (for-next/core), thanks! > > > > > > [1/1] arm64: warn on incorrect placement of the kernel by the bootloader > > > https://git.kernel.org/arm64/c/dd4bc6076587 > > > > > > > This test > > > > ((u64)_text % MIN_KIMG_ALIGN) != 0 > > > > should have been written as > > > > ((u64)_text % MIN_KIMG_ALIGN) != TEXT_OFFSET > > > > because now, we are throwing the warning when > > CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET is enabled :-( > > > > I was reluctant to add another Makefile rule to add a > > -DTEXT_OFFSET=... GCC option when compiling this source file, but it > > appears we need it after all. > > FWIW, I'd be in favour of removing CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET at > this stage as I don't really see the point in it, especially now that > TEXT_OFFSET is on the way out and we've practically forced the use of a > relocatable image. > > What do you think? > I agree. TEXT_OFFSET now always deviates from the default value of 0x80000, and we are only now catching issues that CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET was intended to catch. I'll prepare a patch.
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7f9d38444d6d..16c3f158c80e 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1740,8 +1740,9 @@ config ARM64_DEBUG_PRIORITY_MASKING endif config RELOCATABLE - bool + bool "Build a relocatable kernel image" if EXPERT select ARCH_HAS_RELR + default y help This builds the kernel as a Position Independent Executable (PIE), which retains all relocation metadata required to relocate the diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 3fd2c11c09fc..5d7d6ac034fd 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -319,6 +319,10 @@ void __init setup_arch(char **cmdline_p) xen_early_init(); efi_init(); + + if (!efi_enabled(EFI_BOOT) && ((u64)_text % MIN_KIMG_ALIGN) != 0) + pr_warn(FW_BUG "Kernel image misaligned at boot, please fix your bootloader!"); + arm64_memblock_init(); paging_init();
Commit cfa7ede20f133c ("arm64: set TEXT_OFFSET to 0x0 in preparation for removing it entirely") results in boot failures when booting kernels that are built without KASLR support on broken bootloaders that ignore the TEXT_OFFSET value passed via the header, and use the default of 0x80000 instead. To work around this, turn CONFIG_RELOCATABLE on by default, even if KASLR itself (CONFIG_RANDOMIZE_BASE) is turned off, and require CONFIG_EXPERT to be enabled to deviate from this. Then, emit a warning into the kernel log if we are not booting via the EFI stub (which is permitted to deviate from the placement restrictions) and the kernel base address is not placed according to the rules as laid out in Documentation/arm64/booting.rst. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- v2: use pr_warn() instead of WARN() arch/arm64/Kconfig | 3 ++- arch/arm64/kernel/setup.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-)