Re: [PATCH] arm64: Fix map_range() not splitting mapped blocks

[Marc Zyngier <maz@kernel.org>]

On Mon, 18 Mar 2024 19:35:49 +0000,
Pierre-Clément Tosi <ptosi@google.com> wrote:
> 
> The implementation of map_range() creates the requested mapping by
> walking the page tables, iterating over multiple PTEs and/or descending
> into existing table mappings as needed. When doing so, it assumes any
> pre-existing valid PTE to be a table mapping. This assumption is wrong
> if the platform code attempts to successively map two overlapping ranges
> where the latter intersects a block mapping created for the former.
> 
> As a result, map_range() treats the existing block mapping as a table
> mapping and descends into it i.e. starts interpreting the
> previously-mapped range as an array of PTEs, writing to them and
> potentially even descending further (extra fun with MMIO ranges!).
> 
> Instead, pass any valid non-table mapping to split_block(), which
> ensures that it actually was a block mapping (calls panic() otherwise)
> before splitting it.
> 
> Fixes: 41e2787f5ec4 ("arm64: Reduce add_map() complexity")
> Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
> ---
>  arch/arm/cpu/armv8/cache_v8.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index 697334086f..57d06f0575 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -326,6 +326,8 @@ static void map_range(u64 virt, u64 phys, u64 size, int level,
>  		/* Going one level down */
>  		if (pte_type(&table[i]) == PTE_TYPE_FAULT)
>  			set_pte_table(&table[i], create_table());
> +		else if (pte_type(&table[i]) != PTE_TYPE_TABLE)
> +			split_block(&table[i], level);
>  
>  		next_table = (u64 *)(table[i] & GENMASK_ULL(47, PAGE_SHIFT));
>  		next_size = min(map_size - (virt & (map_size - 1)), size);

This seems pretty reasonable, thanks for looking into this. However, I
can't help but notice that this is done without any BBM, and no TLBI
either.

Are we guaranteed that the updated page tables are not live at the
point of update?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.