| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 20 Apr 2022 10:05:35 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Shanker Donthineni <sdonthineni@...dia.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Vikram Sethi <vsethi@...dia.com>,
Thierry Reding <treding@...dia.com>,
Anshuman Khandual <anshuman.khandual@....com>
Subject: Re: [PATCH] arm64: head: Fix cache inconsistency of the
identity-mapped region
On Fri, 15 Apr 2022 at 19:05, Shanker Donthineni <sdonthineni@...dia.com> wrote:
>
> The secondary cores boot is stuck due to data abort while executing the
> instruction 'ldr x8, =__secondary_switched'. The RELA value of this
> instruction was updated by a primary boot core from __relocate_kernel()
> but those memory updates are not visible to CPUs after calling
> switch_to_vhe() causing problem.
>
> The cacheable/shareable attributes of the identity-mapped regions are
> different while CPU executing in EL1 (MMU enabled) and for a short period
> of time in hyp-stub (EL2-MMU disabled). As per the ARM-ARM specification
> (DDI0487G_b), this is not allowed.
>
> G5.10.3 Cache maintenance requirement:
> "If the change affects the cacheability attributes of the area of memory,
> including any change between Write-Through and Write-Back attributes,
> software must ensure that any cached copies of affected locations are
> removed from the caches, typically by cleaning and invalidating the
> locations from the levels of cache that might hold copies of the locations
> affected by the attribute change."
>
> Clean+invalidate the identity-mapped region till PoC before switching to
> VHE world to fix the cache inconsistency.
>
> Problem analysis with disassembly (vmlinux):
> 1) Both __primary_switch() and enter_vhe() are part of the identity region
> 2) RELA entries and enter_vhe() are sharing the same cache line fff800010970480
> 3) Memory ffff800010970484-ffff800010970498 is updated with EL1-MMU enabled
> 4) CPU fetches intrsuctions of enter_vhe() with EL2-MMU disabled
> - Non-coherent access causing the cache line fff800010970480 drop
> 5) Secondary core executes 'ldr x8, __secondary_switched'
> - Getting data abort because of the incorrect value at ffff800010970488
>
> ffff800010970418 <__primary_switch>:
> ffff800010970418: d503245f bti c
> ffff80001097041c: aa0003f3 mov x19, x0
> ffff800010970420: d5381014 mrs x20, sctlr_el1
> ffff800010970424: 90003c81 adrp x1, ffff800011100000 <init_pg_dir>
> ffff800010970428: 97ffffc4 bl ffff800010970338 <__enable_mmu>
> ffff80001097042c: 97ffffe8 bl ffff8000109703cc <__relocate_kernel>
> ffff800010970430: 58000308 ldr x8, ffff800010970490 <__primary_switch+0x78>
> ffff800010970434: 90ffb480 adrp x0, ffff800010000000 <_text>
> ffff800010970438: d63f0100 blr x8
> ffff80001097043c: d5033fdf isb
> ffff800010970440: d5181014 msr sctlr_el1, x20
> ffff800010970444: d5033fdf isb
> ffff800010970448: 940f7efe bl ffff800010d50040 <__create_page_tables>
> ffff80001097044c: d508871f tlbi vmalle1
> ffff800010970450: d503379f dsb nsh
> ffff800010970454: d5033fdf isb
> ffff800010970458: d5181013 msr sctlr_el1, x19
> ffff80001097045c: d5033fdf isb
> ffff800010970460: d508751f ic iallu
> ffff800010970464: d503379f dsb nsh
> ffff800010970468: d5033fdf isb
> ffff80001097046c: 97ffffd8 bl ffff8000109703cc <__relocate_kernel>
> ffff800010970470: 58000108 ldr x8, ffff800010970490 <__primary_switch+0x78>
> ffff800010970474: 90ffb480 adrp x0, ffff800010000000 <_text>
> ffff800010970478: d61f0100 br x8
> ffff80001097047c: 00df10c8 .word 0x00df10c8
> ffff800010970480: 000dfba8 .word 0x000dfba8
> ...
> ffff800010970498: d51cd041 msr tpidr_el2, x1
> ffff80001097049c: d503201f nop
>
> ffff8000109704a0 <enter_vhe>:
> ffff8000109704a0: d508871f tlbi vmalle1
> ffff8000109704a4: d503379f dsb nsh
> ffff8000109704a8: d5033fdf isb
> ffff8000109704ac: d53d1000 mrs x0, sctlr_el12
> ffff8000109704b0: d5181000 msr sctlr_el1, x0
> ffff8000109704b4: d5033fdf isb
> ffff8000109704b8: d508751f ic iallu
> ffff8000109704bc: d503379f dsb nsh
> ffff8000109704c0: d5033fdf isb
> ffff8000109704c4: d2a60a00 mov x0, #0x30500000
> ffff8000109704c8: f2810000 movk x0, #0x800
> ffff8000109704cc: d51d1000 msr sctlr_el12, x0
> ffff8000109704d0: aa1f03e0 mov x0, xzr
> ffff8000109704d4: d69f03e0 eret
>
> ffff800010961850 <mutate_to_vhe>:
> ffff800010961850: d53c1001 mrs x1, sctlr_el2
> ffff800010961854: 370001c1 tbnz w1, #0, ffff80001096188c <mutate_to_vhe+0x3c>
> ffff800010961858: d5380721 mrs x1, id_aa64mmfr1_el1
> ...
> ffff80001096190c: aa010000 orr x0, x0, x1
> ffff800010961910: d5184000 msr spsr_el1, x0
> ffff800010961914: 14003ae3 b ffff8000109704a0 <enter_vhe>
>
> ffff800010970270 <secondary_startup>:
> ffff800010970270: d503245f bti c
> ffff800010970274: 97dab23a bl ffff80001001cb5c <switch_to_vhe>
> ffff800010970278: 94000049 bl ffff80001097039c <__cpu_secondary_check52bitva>
> ffff80001097027c: 94000145 bl ffff800010970790 <__cpu_setup>
> ffff800010970280: 90001e81 adrp x1, ffff800010d40000 <swapper_pg_dir>
> ffff800010970284: 9400002d bl ffff800010970338 <__enable_mmu>
> ffff800010970288: 58001008 ldr x8, ffff800010970488 <__primary_switch+0x70>
> ffff80001097028c: d61f0100 br x8
>
> Signed-off-by: Shanker Donthineni <sdonthineni@...dia.com>
> ---
> arch/arm64/kernel/head.S | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 6a98f1a38c29a..b5786163697bb 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -462,6 +462,16 @@ SYM_FUNC_START_LOCAL(__primary_switched)
> ldp x29, x30, [sp], #16 // we must enable KASLR, return
> ret // to __primary_switch()
> 0:
> +#endif
> +#ifdef CONFIG_RELOCATABLE
> + /*
> + * Since the RELA entries of the identity-mapped region are updated
> + * with MMU enabled, clean and invalidate those entries to avoid
> + * cache inconsistency while accessing with MMU disabled in hyp-stub.
> + */
> + adrp x0, __idmap_text_start
> + adr_l x1, __idmap_text_end
> + bl dcache_clean_inval_poc
> #endif
> bl switch_to_vhe // Prefer VHE if possible
> ldp x29, x30, [sp], #16
Thanks for the elaborate report.
I'd prefer to fix this by moving the literal out of the ID map
entirely. Does the below also fix your issue?
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 6a98f1a38c29..97134d6f78ff 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -639,10 +639,15 @@ SYM_FUNC_START_LOCAL(secondary_startup)
bl __cpu_setup // initialise processor
adrp x1, swapper_pg_dir
bl __enable_mmu
- ldr x8, =__secondary_switched
+ ldr_l x8, .L__secondary_switched
br x8
SYM_FUNC_END(secondary_startup)
+ .pushsection ".rodata", "a", %progbits
+.L__secondary_switched:
+ .quad __secondary_switched
+ .popsection
+
SYM_FUNC_START_LOCAL(__secondary_switched)
adr_l x5, vectors
msr vbar_el1, x5
Powered by blists - more mailing lists