Date: Mon, 14 Aug 2017 15:49:15 +0100 From: Ard Biesheuvel <ard.biesheuvel@...aro.org> To: Dave Martin <Dave.Martin@....com> Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, Mark Rutland <mark.rutland@....com>, Kees Cook <keescook@...omium.org>, Arnd Bergmann <arnd@...db.de>, Nicolas Pitre <nico@...aro.org>, Marc Zyngier <marc.zyngier@....com>, Russell King <linux@...linux.org.uk>, Tony Lindgren <tony@...mide.com>, Matt Fleming <matt@...eblueprint.co.uk>, Thomas Garnier <thgarnie@...gle.com>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org> Subject: Re: [PATCH 17/30] arm-soc: tegra: make sleep asm code runtime relocatable On 14 August 2017 at 15:42, Dave Martin <Dave.Martin@....com> wrote: > On Mon, Aug 14, 2017 at 01:53:58PM +0100, Ard Biesheuvel wrote: >> The PIE kernel build does not allow absolute references encoded in >> movw/movt instruction pairs, so use our mov_l macro instead (which >> will still use such a pair unless CONFIG_RELOCATABLE is defined) >> >> Also, avoid 32-bit absolute literals to refer to absolute symbols. >> Instead, use a 16 bit reference so that PIE linker cannot get >> confused whether the symbol reference is subject to relocation at >> runtime. > > This sounds like we're papering over something. > > If the linker is "confused", that sounds like we are either abusing > it somehow, or the linker is broken. > There is some ambiguity in how SHN_ABS symbols are treated in shared libraries and PIE executables. https://sourceware.org/ml/binutils/2012-05/msg00019.html I haven't confirmed whether it actually causes problems in this particular case, but it is safer (and not entirely inappropriate) to use a 16-bit field for a quantity that can easily fit one. > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org> >> --- >> arch/arm/mach-tegra/sleep-tegra20.S | 22 ++++++++++++-------- >> arch/arm/mach-tegra/sleep-tegra30.S | 6 +++--- >> arch/arm/mach-tegra/sleep.S | 4 ++-- >> 3 files changed, 18 insertions(+), 14 deletions(-) >> >> diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S >> index 5c8e638ee51a..cab95de5c8f1 100644 >> --- a/arch/arm/mach-tegra/sleep-tegra20.S >> +++ b/arch/arm/mach-tegra/sleep-tegra20.S >> @@ -99,7 +99,7 @@ ENTRY(tegra20_cpu_shutdown) >> cmp r0, #0 >> reteq lr @ must not be called for CPU 0 >> mov32 r1, TEGRA_IRAM_RESET_BASE_VIRT >> - ldr r2, =__tegra20_cpu1_resettable_status_offset >> + ldrh r2, 0f >> mov r12, #CPU_RESETTABLE >> strb r12, [r1, r2] >> >> @@ -121,6 +121,7 @@ ENTRY(tegra20_cpu_shutdown) >> beq . >> ret lr >> ENDPROC(tegra20_cpu_shutdown) >> +0: .short __tegra20_cpu1_resettable_status_offset >> #endif >> >> #ifdef CONFIG_PM_SLEEP >> @@ -181,6 +182,9 @@ ENTRY(tegra_pen_unlock) >> ret lr >> ENDPROC(tegra_pen_unlock) >> >> +.L__tegra20_cpu1_resettable_status_offset: >> + .short __tegra20_cpu1_resettable_status_offset >> + >> /* >> * tegra20_cpu_clear_resettable(void) >> * >> @@ -189,7 +193,7 @@ ENDPROC(tegra_pen_unlock) >> */ >> ENTRY(tegra20_cpu_clear_resettable) >> mov32 r1, TEGRA_IRAM_RESET_BASE_VIRT > > Can we simply port mov32 to mov_l? Or do we hit a problem with > multiplatform kernels where mov_l may involve a literal pool entry > (and does it matter)? > The only place where it matters is in code that lives in idmap.text, since the relative reference will point to the ID mapped alias of a section that is not covered by the ID ID map. I think we should be able to use mov_l everywhere else, and it should do the right thing for ordinary and PIE builds
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.