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

Gabriel Quintáns Souto posted 1 patch 3 weeks ago
There is a newer version of this series
xen/arch/arm/mmu/pt.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
[PATCH v1] xen/arm: validate non-common page-table ranges
Posted by Gabriel Quintáns Souto 3 weeks ago
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@gmail.com>
---
 xen/arch/arm/mmu/pt.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/mmu/pt.c b/xen/arch/arm/mmu/pt.c
index 621b47d..ad0d1e6 100644
--- a/xen/arch/arm/mmu/pt.c
+++ b/xen/arch/arm/mmu/pt.c
@@ -13,6 +13,7 @@
 
 #include <asm/current.h>
 #include <asm/fixmap.h>
+#include <asm/mmu/layout.h>
 
 #ifdef NDEBUG
 static inline void
@@ -589,6 +590,29 @@ 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,
@@ -601,15 +625,13 @@ static int xen_pt_update(unsigned long virt,
     unsigned long vfn = virt >> PAGE_SHIFT;
     unsigned long left = nr_mfns;
 
-    /*
-     * For arm32, page-tables are different on each CPUs. Yet, they share
-     * some common mappings. It is assumed that only common mappings
-     * will be modified with this function.
-     *
-     * XXX: Add a check.
-     */
     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) )
+        return -EINVAL;
+    #endif
+
     if ( flags_has_rwx(flags) )
     {
         mm_printk("Mappings should not be both Writeable and Executable.\n");
-- 
2.54.0


Re: [PATCH v1] xen/arm: validate non-common page-table ranges
Posted by Alejandro Vallejo 2 weeks, 5 days ago
On Fri Oct 2, 2026 at 9:14 PM CEST, 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@gmail.com>

I'm not an arm person, so take this with a serious pinch of salt.

> ---
>  xen/arch/arm/mmu/pt.c | 36 +++++++++++++++++++++++++++++-------
>  1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/mmu/pt.c b/xen/arch/arm/mmu/pt.c
> index 621b47d..ad0d1e6 100644
> --- a/xen/arch/arm/mmu/pt.c
> +++ b/xen/arch/arm/mmu/pt.c
> @@ -13,6 +13,7 @@
>  
>  #include <asm/current.h>
>  #include <asm/fixmap.h>
> +#include <asm/mmu/layout.h>
>  
>  #ifdef NDEBUG
>  static inline void
> @@ -589,6 +590,29 @@ 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,

We typically prefer positive helpers so you never end up with
!xen_pt_range_is_non_common(), which is awkward to read.

IMO, this should be xen_pt_range_is_common() with the reverse polarity
in the code elsewhere.

> +                                       unsigned long size)
> +{
> +    unsigned long temporary_start = TEMPORARY_AREA_ADDR(0);
> +    unsigned long temporary_size = XEN_PT_LEVEL_SIZE(1);
> +

All ifdefs can go away if you do:

  if ( !IS_ENABLED(CONFIG_ARM_32) )
      return 0;

... here. Everything else drops with DCE when compiling with ARM_32=n
both with DEBUG=y and DEBUG=n.

And codegen would be identical.

> +    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,
> @@ -601,15 +625,13 @@ static int xen_pt_update(unsigned long virt,
>      unsigned long vfn = virt >> PAGE_SHIFT;
>      unsigned long left = nr_mfns;
>  
> -    /*
> -     * For arm32, page-tables are different on each CPUs. Yet, they share
> -     * some common mappings. It is assumed that only common mappings
> -     * will be modified with this function.
> -     *
> -     * XXX: Add a check.
> -     */
>      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) )

nit: Perhaps ASSERT_UNREACHABLE() here? Presumably this path is a
logical error.

> +        return -EINVAL;
> +    #endif
> +
>      if ( flags_has_rwx(flags) )
>      {
>          mm_printk("Mappings should not be both Writeable and Executable.\n");

Cheers,
Alejandro