* [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.