linux-kernel - Re: [PATCH] arm64: mm: Ensure phys_to_ttbr on pgdir for idmap_cpu_replace

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aH-mlN88NrTzahfM@J2N7QTR9R3>
Date: Tue, 22 Jul 2025 15:56:20 +0100
From: Mark Rutland <mark.rutland@....com>
To: Weikang Guo <guoweikang.kernel@...il.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
	Anshuman Khandual <anshuman.khandual@....com>,
	Ard Biesheuvel <ardb@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: mm: Ensure phys_to_ttbr on pgdir for
 idmap_cpu_replace_ttbr1

On Tue, Jul 22, 2025 at 04:21:13PM +0800, Weikang Guo wrote:
> Commit 5ffdfaedfa0a ("arm64: mm: Support Common Not Private translations")
> changed the contract of idmap_cpu_replace_ttbr1, requiring that the TTBR
> argument passed in should already be processed by phys_to_ttbr (i.e., in
> TTBR format, not just a raw physical address).
> 
> However, the current map_kernel implementation does not always convert the
> pgdir/ttbr argument via phys_to_ttbr before calling
> idmap_cpu_replace_ttbr1. This can lead to issues on systems with
> CONFIG_ARM64_PA_BITS_52 enabled, as the TTBR would not be properly folded
> per the ARMv8.2+ requirements.

For the cases below I don't believe that this is actually a problem.
Since commit:

  453dfcee70c5c344 ("arm64: booting: Require placement within 48-bit addressable memory")

... we require that the kernel Image (including any trailing unallocated
bytes accounted for in image_size) are below the 48-bit address limit,
and so there should be no difference between the PA and TTBR format.

We could probably test and enforce that in the early boot code somehow,
if we're not doing that already.

If we were going to change things to avoid accidents in future, I think
it would be better to enforce this with the type system. e.g. we could
have a ttbr_val type that's distinct from phys_addr_t. Even then, for
the idmap code I think it's better to avoid the phys_to_ttbr() dance,
since that has runtime patching.

Mark.

> 
> Signed-off-by: Weikang Guo <guoweikang.kernel@...il.com>
> 
> ---
> Note: I do not currently have access to ARM64 hardware or an emulation
> environment that supports 52-bit physical address (PA52). I would
> greatly appreciate if anyone with such a platform could help test
> this patch. Thank you!
> ---
>  arch/arm64/kernel/pi/map_kernel.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
> index 0f4bd7771859..05a04eb91e81 100644
> --- a/arch/arm64/kernel/pi/map_kernel.c
> +++ b/arch/arm64/kernel/pi/map_kernel.c
> @@ -18,7 +18,7 @@
>  
>  extern const u8 __eh_frame_start[], __eh_frame_end[];
>  
> -extern void idmap_cpu_replace_ttbr1(void *pgdir);
> +extern void idmap_cpu_replace_ttbr1(phys_addr_t);
>  
>  static void __init map_segment(pgd_t *pg_dir, u64 *pgd, u64 va_offset,
>  			       void *start, void *end, pgprot_t prot,
> @@ -90,7 +90,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
>  		    true, root_level);
>  	dsb(ishst);
>  
> -	idmap_cpu_replace_ttbr1(init_pg_dir);
> +	idmap_cpu_replace_ttbr1(phys_to_ttbr((u64)init_pg_dir));
>  
>  	if (twopass) {
>  		if (IS_ENABLED(CONFIG_RELOCATABLE))
> @@ -129,7 +129,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
>  	/* Copy the root page table to its final location */
>  	memcpy((void *)swapper_pg_dir + va_offset, init_pg_dir, PAGE_SIZE);
>  	dsb(ishst);
> -	idmap_cpu_replace_ttbr1(swapper_pg_dir);
> +	idmap_cpu_replace_ttbr1(phys_to_ttbr((u64)swapper_pg_dir));
>  }
>  
>  static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
> -- 
> 2.25.1
> 
>