Re: [PATCH v3] xen/arm: validate non-common page-table ranges

["Orzel, Michal" <michal.orzel@xxxxxxx>]

This patch does not apply and it looks as if it was diffed against your previous
revision, not the staging branch. Please send the correct patch.

~Michal

On 05-Oct-26 19:59, Gabriel Quintáns Souto wrote:
> On arm32, page-tables are different on each CPUs, although they share some
> common mappings.
> 
> The xen_pt_update function assumed that only said mappings would be
> modified, but did not check this assumption.
> 
> Add a range check to reject modifications to non-common mappings.
> 
> Signed-off-by: Gabriel Quintáns Souto <gabi.qs.mail@xxxxxxxxx>
> ---
> v3:
>  - Dropped accidentally tracked patch file from commit.
> 
> v2:
> - Inverted polarity of xen_pt_range_is_non_common() as suggested.
> - Moved xen_pt_range_is_common() implementation to arch-specific headers
> (arm32 vs arm64) instead of keeping #ifdef CONFIG_ARM_32 in pt.c (tried
> IS_ENABLED() for DCE, but ARM64 lacks definitions for
> DOMHEAP_VIRT_START/TEMPORARY_AREA_ADDR, breaking the build).
> - Added ASSERT_UNREACHABLE() on failure path in xen_pt_update().
> ---
>  xen/arch/arm/include/asm/arm32/mm.h | 24 +++++++++++++++++++++++
>  xen/arch/arm/include/asm/arm64/mm.h | 10 ++++++++++
>  xen/arch/arm/mmu/pt.c               | 30 ++++-------------------------
>  3 files changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/mm.h 
> b/xen/arch/arm/include/asm/arm32/mm.h
> index 856f2db..c7d09d3 100644
> --- a/xen/arch/arm/include/asm/arm32/mm.h
> +++ b/xen/arch/arm/include/asm/arm32/mm.h
> @@ -16,6 +16,30 @@ static inline bool arch_mfns_in_directmap(unsigned long 
> mfn, unsigned long nr)
>      return false;
>  }
>  
> +static inline bool ranges_overlap(unsigned long start1, unsigned long size1,
> +                           unsigned long start2, unsigned long size2)
> +{
> +    if ( start1 < start2 )
> +        return start2 - start1 < size1;
> +
> +    return start1 - start2 < size2;
> +}
> +
> +/*
> + * On ARM32, page-tables differ between CPUs, so not all mappings are common.
> + */
> +static inline bool xen_pt_range_is_common(unsigned long virt,
> +                                       unsigned long size)
> +{
> +    unsigned long temporary_start = TEMPORARY_AREA_ADDR(0);
> +    unsigned long temporary_size = XEN_PT_LEVEL_SIZE(1);
> +
> +    return !ranges_overlap(virt, size,
> +                          DOMHEAP_VIRT_START, DOMHEAP_VIRT_SIZE) &&
> +           !ranges_overlap(virt, size,
> +                          temporary_start, temporary_size);
> +}
> +
>  bool init_domheap_mappings(unsigned int cpu);
>  
>  static inline void arch_setup_page_tables(void)
> diff --git a/xen/arch/arm/include/asm/arm64/mm.h 
> b/xen/arch/arm/include/asm/arm64/mm.h
> index b4f7545..a3cb56d 100644
> --- a/xen/arch/arm/include/asm/arm64/mm.h
> +++ b/xen/arch/arm/include/asm/arm64/mm.h
> @@ -12,6 +12,16 @@ static inline bool arch_mfns_in_directmap(unsigned long 
> mfn, unsigned long nr)
>      return true;
>  }
>  
> +/*
> + * On ARM64, the Xen page-tables are shared by all the CPUs,
> + * any range is therefore common.
> + */
> +static inline bool xen_pt_range_is_common(unsigned long virt,
> +                                          unsigned long size)
> +{
> +    return true;
> +}
> +
>  void arch_setup_page_tables(void);
>  
>  void update_boot_mapping(bool enable);
> diff --git a/xen/arch/arm/mmu/pt.c b/xen/arch/arm/mmu/pt.c
> index ad0d1e6..5e1b427 100644
> --- a/xen/arch/arm/mmu/pt.c
> +++ b/xen/arch/arm/mmu/pt.c
> @@ -590,29 +590,6 @@ static unsigned int xen_pt_check_contig(unsigned long 
> vfn, mfn_t mfn,
>      return XEN_PT_4K_NR_CONTIG;
>  }
>  
> -#ifdef CONFIG_ARM_32
> -static bool ranges_overlap(unsigned long start1, unsigned long size1,
> -                           unsigned long start2, unsigned long size2)
> -{
> -    if ( start1 < start2 )
> -        return start2 - start1 < size1;
> -
> -    return start1 - start2 < size2;
> -}
> -
> -static bool xen_pt_range_is_non_common(unsigned long virt,
> -                                       unsigned long size)
> -{
> -    unsigned long temporary_start = TEMPORARY_AREA_ADDR(0);
> -    unsigned long temporary_size = XEN_PT_LEVEL_SIZE(1);
> -
> -    return ranges_overlap(virt, size,
> -                          DOMHEAP_VIRT_START, DOMHEAP_VIRT_SIZE) ||
> -           ranges_overlap(virt, size,
> -                          temporary_start, temporary_size);
> -}
> -#endif
> -
>  static DEFINE_SPINLOCK(xen_pt_lock);
>  
>  static int xen_pt_update(unsigned long virt,
> @@ -627,10 +604,11 @@ static int xen_pt_update(unsigned long virt,
>  
>      const mfn_t root = maddr_to_mfn(READ_SYSREG64(TTBR0_EL2));
>  
> -    #ifdef CONFIG_ARM_32
> -    if ( xen_pt_range_is_non_common(virt, nr_mfns * PAGE_SIZE) )
> +    if ( !xen_pt_range_is_common(virt, nr_mfns * PAGE_SIZE) )
> +    {
> +        ASSERT_UNREACHABLE();
>          return -EINVAL;
> -    #endif
> +    }
>  
>      if ( flags_has_rwx(flags) )
>      {