[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMj1kXG30L5+pXco69k2M2=rw6L4wTvYARL8WUs5nfdC-3nK+g@mail.gmail.com>
Date: Mon, 30 May 2022 15:52:39 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Sasha Levin <sashal@...nel.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"# 3.4.x" <stable@...r.kernel.org>,
Russell King <rmk+kernel@...linux.org.uk>,
Russell King <linux@...linux.org.uk>,
Linus Walleij <linus.walleij@...aro.org>,
Nicolas Pitre <nico@...xnic.net>,
Keith Packard <keithpac@...zon.com>,
Arnd Bergmann <arnd@...db.de>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH AUTOSEL 5.4 51/55] ARM: 9201/1: spectre-bhb: rely on
linker to emit cross-section literal loads
AUTONAK
On Mon, 30 May 2022 at 15:49, Sasha Levin <sashal@...nel.org> wrote:
>
> From: Ard Biesheuvel <ardb@...nel.org>
>
> [ Upstream commit ad12c2f1587c6ec9b52ff226f438955bfae6ad89 ]
>
> The assembler does not permit 'LDR PC, <sym>' when the symbol lives in a
> different section, which is why we have been relying on rather fragile
> open-coded arithmetic to load the address of the vector_swi routine into
> the program counter using a single LDR instruction in the SWI slot in
> the vector table. The literal was moved to a different section to in
> commit 19accfd373847 ("ARM: move vector stubs") to ensure that the
> vector stubs page does not need to be mapped readable for user space,
> which is the case for the vector page itself, as it carries the kuser
> helpers as well.
>
> So the cross-section literal load is open-coded, and this relies on the
> address of vector_swi to be at the very start of the vector stubs page,
> and we won't notice if we got it wrong until booting the kernel and see
> it break. Fortunately, it was guaranteed to break, so this was fragile
> but not problematic.
>
> Now that we have added two other variants of the vector table, we have 3
> occurrences of the same trick, and so the size of our ISA/compiler/CPU
> validation space has tripled, in a way that may cause regressions to only
> be observed once booting the image in question on a CPU that exercises a
> particular vector table.
>
> So let's switch to true cross section references, and let the linker fix
> them up like it fixes up all the other cross section references in the
> vector page.
>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
> arch/arm/kernel/entry-armv.S | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 8e8efe28d799..4d900c61a0f7 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -1074,10 +1074,15 @@ ENDPROC(vector_bhb_bpiall_\name)
> .endm
>
> .section .stubs, "ax", %progbits
> - @ This must be the first word
> + @ These need to remain at the start of the section so that
> + @ they are in range of the 'SWI' entries in the vector tables
> + @ located 4k down.
> +.L__vector_swi:
> .word vector_swi
> #ifdef CONFIG_HARDEN_BRANCH_HISTORY
> +.L__vector_bhb_loop8_swi:
> .word vector_bhb_loop8_swi
> +.L__vector_bhb_bpiall_swi:
> .word vector_bhb_bpiall_swi
> #endif
>
> @@ -1220,10 +1225,11 @@ vector_addrexcptn:
> .globl vector_fiq
>
> .section .vectors, "ax", %progbits
> -.L__vectors_start:
> W(b) vector_rst
> W(b) vector_und
> - W(ldr) pc, .L__vectors_start + 0x1000
> +ARM( .reloc ., R_ARM_LDR_PC_G0, .L__vector_swi )
> +THUMB( .reloc ., R_ARM_THM_PC12, .L__vector_swi )
> + W(ldr) pc, .
> W(b) vector_pabt
> W(b) vector_dabt
> W(b) vector_addrexcptn
> @@ -1232,10 +1238,11 @@ vector_addrexcptn:
>
> #ifdef CONFIG_HARDEN_BRANCH_HISTORY
> .section .vectors.bhb.loop8, "ax", %progbits
> -.L__vectors_bhb_loop8_start:
> W(b) vector_rst
> W(b) vector_bhb_loop8_und
> - W(ldr) pc, .L__vectors_bhb_loop8_start + 0x1004
> +ARM( .reloc ., R_ARM_LDR_PC_G0, .L__vector_bhb_loop8_swi )
> +THUMB( .reloc ., R_ARM_THM_PC12, .L__vector_bhb_loop8_swi )
> + W(ldr) pc, .
> W(b) vector_bhb_loop8_pabt
> W(b) vector_bhb_loop8_dabt
> W(b) vector_addrexcptn
> @@ -1243,10 +1250,11 @@ vector_addrexcptn:
> W(b) vector_bhb_loop8_fiq
>
> .section .vectors.bhb.bpiall, "ax", %progbits
> -.L__vectors_bhb_bpiall_start:
> W(b) vector_rst
> W(b) vector_bhb_bpiall_und
> - W(ldr) pc, .L__vectors_bhb_bpiall_start + 0x1008
> +ARM( .reloc ., R_ARM_LDR_PC_G0, .L__vector_bhb_bpiall_swi )
> +THUMB( .reloc ., R_ARM_THM_PC12, .L__vector_bhb_bpiall_swi )
> + W(ldr) pc, .
> W(b) vector_bhb_bpiall_pabt
> W(b) vector_bhb_bpiall_dabt
> W(b) vector_addrexcptn
> --
> 2.35.1
>
Powered by blists - more mailing lists