Re: [PATCH AUTOSEL 5.18 147/159] ARM: 9201/1: spectre-bhb: rely on linker to emit cross-section literal loads

Ard Biesheuvel
2022-05-30T13:41:45Z

AUTONAK

As discussed before, please disregard all patches authored by me when
running the bot.

On Mon, 30 May 2022 at 15:31, Sasha Levin <sashal@kernel.org> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.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@kernel.org>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Sasha Levin <sashal@kernel.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 7a8682468a84..f181af18a904 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -1176,10 +1176,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
>
> @@ -1322,10 +1327,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
> @@ -1334,10 +1340,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
> @@ -1345,10 +1352,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
>