All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] xen/arm: validate non-common page-table ranges
@ 2026-11-10 14:52 Gabriel Quintáns Souto
  2026-11-12 14:26 ` Orzel, Michal
  0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Quintáns Souto @ 2026-11-10 14:52 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, michal.orzel,
	Volodymyr_Babchuk, Gabriel Quintáns Souto

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>
---
v4:
 - Rebased onto staging (v3 was diffed against v2).

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               | 14 +++++++-------
 3 files changed, 41 insertions(+), 7 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 81fe121..42fa79a 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
@@ -601,15 +602,14 @@ 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));
 
+    if ( !xen_pt_range_is_common(virt, nr_mfns * PAGE_SIZE) )
+    {
+        ASSERT_UNREACHABLE();
+        return -EINVAL;
+    }
+
     if ( flags_has_rwx(flags) )
     {
         mm_printk("Mappings should not be both Writeable and Executable.\n");
-- 
2.56.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] xen/arm: validate non-common page-table ranges
  2026-11-10 14:52 [PATCH v4] xen/arm: validate non-common page-table ranges Gabriel Quintáns Souto
@ 2026-11-12 14:26 ` Orzel, Michal
  2026-11-14  1:53   ` Gabriel Quintáns Souto
  0 siblings, 1 reply; 3+ messages in thread
From: Orzel, Michal @ 2026-11-12 14:26 UTC (permalink / raw)
  To: Gabriel Quintáns Souto, xen-devel
  Cc: sstabellini, julien, bertrand.marquis, Volodymyr_Babchuk



On 10-Nov-26 15:52, Gabriel Quintáns Souto wrote:
> On arm32, page-tables are different on each CPUs, although they share some
> common mappings.
Please say that the non-common area is the per-CPU domheap mapping.

> 
> 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>
> ---
> v4:
>  - Rebased onto staging (v3 was diffed against v2).
> 
> 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               | 14 +++++++-------
>  3 files changed, 41 insertions(+), 7 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);
There's no need to check for TEMPORARY area. It's only ever used in boot
pgtables, not in runtime tables. Therefore what we need is just the following:
virt < DOMHEAP_VIRT_START && size <= DOMHEAP_VIRT_START - virt
without any helper.

BTW. This patch breaks Arm32 MPU build. If at all, the helper should be placed
under mmu/.


> +    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 81fe121..42fa79a 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>
Is this needed?

~Michal



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] xen/arm: validate non-common page-table ranges
  2026-11-12 14:26 ` Orzel, Michal
@ 2026-11-14  1:53   ` Gabriel Quintáns Souto
  0 siblings, 0 replies; 3+ messages in thread
From: Gabriel Quintáns Souto @ 2026-11-14  1:53 UTC (permalink / raw)
  To: Orzel, Michal, Gabriel Quintáns Souto, xen-devel
  Cc: sstabellini, julien, bertrand.marquis, Volodymyr_Babchuk

> There's no need to check for TEMPORARY area. It's only ever used in boot
> pgtables, not in runtime tables. Therefore what we need is just the following:
> virt < DOMHEAP_VIRT_START && size <= DOMHEAP_VIRT_START - virt
> without any helper.
>
> BTW. This patch breaks Arm32 MPU build. If at all, the helper should be placed
> under mmu/.

Ok, thanks for the review. I was not really sure whether TEMPORARY_AREA was
needed or not, so thanks for making that clear. One thing, though:
`virt < DOMHEAP_VIRT_START && size <= DOMHEAP_VIRT_START - virt`

To do that validation in pt.c, I would need to use `#ifdef CONFIG_ARM_32`
instead of `IS_ENABLED()` (which would let DCE drop it on arm64), otherwise it
doesn't build on arm64. That's why I made the xen_pt_range_is_common helper. How
should I approach this then? I'll drop `ranges_overlap()` either way, but
should I keep `xen_pt_range_is_common()` and move it under `mmu/`, or just drop
all the helpers and use `#ifdef CONFIG_ARM_32` directly in `pt.c`? I assume a
single `#ifdef` in `pt.c` is fine, so I'd go with the second option, as you
suggested. Let me know what you think.

>> +#include <asm/mmu/layout.h>
> Is this needed?

I think I added it while trying to get the helper to build, but I think
it's already pulled in through asm/config.h, so I'll likely drop it in v5.

Best regards,
Gabriel

On Thu Nov 12, 2026 at 3:26 PM CET, Michal Orzel wrote:
>
>
> On 10-Nov-26 15:52, Gabriel Quintáns Souto wrote:
>> On arm32, page-tables are different on each CPUs, although they share some
>> common mappings.
> Please say that the non-common area is the per-CPU domheap mapping.
>
>> 
>> 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>
>> ---
>> v4:
>>  - Rebased onto staging (v3 was diffed against v2).
>> 
>> 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               | 14 +++++++-------
>>  3 files changed, 41 insertions(+), 7 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);
> There's no need to check for TEMPORARY area. It's only ever used in boot
> pgtables, not in runtime tables. Therefore what we need is just the following:
> virt < DOMHEAP_VIRT_START && size <= DOMHEAP_VIRT_START - virt
> without any helper.
>
> BTW. This patch breaks Arm32 MPU build. If at all, the helper should be placed
> under mmu/.
>
>
>> +    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 81fe121..42fa79a 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>
> Is this needed?
>
> ~Michal



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-14  1:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10 14:52 [PATCH v4] xen/arm: validate non-common page-table ranges Gabriel Quintáns Souto
2026-11-12 14:26 ` Orzel, Michal
2026-11-14  1:53   ` Gabriel Quintáns Souto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.