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