All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV
@ 2026-10-31  0:33 Jason Andryuk
  2026-10-31  0:45 ` sashiko-bot
  2026-10-31  3:20 ` Borislav Petkov
  0 siblings, 2 replies; 3+ messages in thread
From: Jason Andryuk @ 2026-10-31  0:33 UTC (permalink / raw)
  To: Juergen Gross, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Andy Lutomirski, Peter Zijlstra, Boris Ostrovsky, Yazen Ghannam,
	Mario Limonciello
  Cc: Jason Andryuk, Sean Christopherson, Ingo Molnar, virtualization,
	linux-kernel, xen-devel

Xen PV guests update page tables via hypercalls, and Xen refuses to let a
domain map machine frames it does not own, such as chipset MMIO in a domU.
The set_pte() paths ignore the hypercall result, so ioremap() returns a
mapping whose PTEs are not present.  Accessing the virtual address then
faults.  This was seen in print_s5_reset_status_mmio() on Xen PV domUs
on AMD Zen hosts.  The machine address is not accessible by the domU and
faults on access.

Add a pv_mmu_ops hook, ioremap_verify_range, which __ioremap_caller()
calls after ioremap_page_range().  It defaults to returning success.
Xen PV implements it by walking the range with lookup_address() and
failing if any entry is not present, making ioremap() return NULL.

This also affects a Xen PV dom0: an ioremap() of a range containing any
page Xen refuses to map (e.g. MMIO Xen reserves for itself) now fails as
a whole, rather than succeeding and faulting only when the refused page
is accessed.  Pages Xen forces read-only are still mapped present and
are unaffected.

Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
Link: https://lore.kernel.org/xen-devel/20251219010131.12659-1-ariadne@ariadne.space/
Suggested-by: Sean Christopherson <seanjc@google.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 arch/x86/include/asm/paravirt.h       | 10 ++++++++++
 arch/x86/include/asm/paravirt_types.h |  6 ++++++
 arch/x86/kernel/paravirt.c            |  7 +++++++
 arch/x86/mm/ioremap.c                 |  3 +++
 arch/x86/xen/mmu_pv.c                 | 24 ++++++++++++++++++++++++
 5 files changed, 50 insertions(+)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 0591aa38fd85..30cb1d1636e4 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -495,6 +495,11 @@ static inline void arch_leave_lazy_mmu_mode(void)
 	arch_flush_lazy_mmu_mode();
 }
 
+static inline int ioremap_verify_range(unsigned long addr, unsigned long size)
+{
+	return PVOP_CALL2(int, pv_ops, mmu.ioremap_verify_range, addr, size);
+}
+
 static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
 				phys_addr_t phys, pgprot_t flags)
 {
@@ -557,6 +562,11 @@ static __always_inline unsigned long arch_local_irq_save(void)
 static inline void paravirt_enter_mmap(struct mm_struct *mm)
 {
 }
+
+static inline int ioremap_verify_range(unsigned long addr, unsigned long size)
+{
+	return 0;
+}
 #endif
 
 #ifndef CONFIG_PARAVIRT
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index b4c4a23e77a1..674613301a83 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -164,6 +164,12 @@ struct pv_mmu_ops {
 
 	void (*lazy_mode_flush)(void);
 
+	/*
+	 * Verify that an ioremap()ed kernel virtual address range was
+	 * actually mapped.  Returns 0 on success.
+	 */
+	int (*ioremap_verify_range)(unsigned long addr, unsigned long size);
+
 	/* dom0 ops */
 
 	/* Sometimes the physical address is a pfn, and sometimes its
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 00b59d774389..b1bc519c660d 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -88,6 +88,11 @@ static noinstr void pv_native_set_debugreg(int regno, unsigned long val)
 {
 	native_set_debugreg(regno, val);
 }
+
+static int native_ioremap_verify_range(unsigned long addr, unsigned long size)
+{
+	return 0;
+}
 #endif
 
 struct pv_info pv_info = {
@@ -207,6 +212,8 @@ struct paravirt_patch_template pv_ops = {
 
 	.mmu.lazy_mode_flush	= paravirt_nop,
 
+	.mmu.ioremap_verify_range	= native_ioremap_verify_range,
+
 	.mmu.set_fixmap		= native_set_fixmap,
 #endif /* CONFIG_PARAVIRT_XXL */
 };
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 12c8180ca1ba..6b45a7eef593 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -301,6 +301,9 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
 	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot))
 		goto err_free_area;
 
+	if (ioremap_verify_range(vaddr, size))
+		goto err_free_area;
+
 	ret_addr = (void __iomem *) (vaddr + offset);
 	mmiotrace_ioremap(unaligned_phys_addr, unaligned_size, ret_addr);
 
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 377c3c0d9f20..e524d318171f 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -359,6 +359,29 @@ static void xen_set_pte(pte_t *ptep, pte_t pteval)
 	__xen_set_pte(ptep, pteval);
 }
 
+/*
+ * PTE updates are made via (possibly batched) hypercalls whose failures
+ * are not propagated back to the caller.  Xen refuses to let a domain map
+ * machine frames it does not own (e.g. chipset MMIO in a domU), leaving the
+ * PTE not present.  Walk the range to make sure every page got mapped.
+ */
+static int xen_ioremap_verify_range(unsigned long addr, unsigned long size)
+{
+	unsigned long end = addr + size;
+	unsigned int level;
+	pte_t *pte;
+
+	while (addr < end) {
+		pte = lookup_address(addr, &level);
+		if (!pte || !pte_present(*pte))
+			return -ENXIO;
+
+		addr = (addr & page_level_mask(level)) + page_level_size(level);
+	}
+
+	return 0;
+}
+
 static pte_t xen_ptep_modify_prot_start(struct vm_area_struct *vma,
 					unsigned long addr, pte_t *ptep)
 {
@@ -2218,6 +2241,7 @@ void __init xen_init_mmu_ops(void)
 	pv_ops.mmu.enter_mmap = xen_enter_mmap;
 	pv_ops.mmu.exit_mmap = xen_exit_mmap;
 	pv_ops.mmu.lazy_mode_flush = xen_flush_lazy_mmu;
+	pv_ops.mmu.ioremap_verify_range = xen_ioremap_verify_range;
 	pv_ops.mmu.set_fixmap = xen_set_fixmap;
 
 	memset(dummy_mapping, 0xff, PAGE_SIZE);
-- 
2.34.1


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

* Re: [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV
  2026-10-31  0:33 [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV Jason Andryuk
@ 2026-10-31  0:45 ` sashiko-bot
  2026-10-31  3:20 ` Borislav Petkov
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-31  0:45 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] x86/ioremap: KMSAN shadow/origin page leak in __ioremap_caller()
--

commit 17e6df6da515e9dac650cd6369fda70f312c2a6a
Author: Jason Andryuk <jason.andryuk@amd.com>

x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV

This commit adds an ioremap_verify_range hook to pv_mmu_ops so __ioremap_caller() can verify that page table entries were actually mapped after ioremap_page_range(). On Xen PV, it walks the virtual address range with lookup_address() and fails if any PTE is not present, causing ioremap() to return NULL when Xen rejects mapping machine frames.

Link: https://lore.kernel.org/xen-devel/20251219010131.12659-1-ariadne@ariadne.space/

> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 12c8180ca1ba7..6b45a7eef5934 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -301,6 +301,9 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
>  	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot))
>  		goto err_free_area;
>  
> +	if (ioremap_verify_range(vaddr, size))
> +		goto err_free_area;

[Severity: Medium]
When CONFIG_KMSAN=y is enabled, can failing ioremap_verify_range() in
__ioremap_caller() leak the KMSAN shadow and origin pages allocated by
ioremap_page_range()?

In ioremap_page_range() -> vmap_page_range(), kmsan_ioremap_page_range()
allocates and maps order-1 shadow and origin pages (which are normal guest
RAM and succeed on Xen PV even when Xen refuses the MMIO PTEs):

mm/vmalloc.c:vmap_page_range() {
	err = vmap_range_noflush(addr, end, phys_addr, pgprot_nx(prot),
				 ioremap_max_page_shift);
	flush_cache_vmap(addr, end);
	if (!err)
		err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,
					       ioremap_max_page_shift);
	return err;
}

When ioremap_verify_range() subsequently fails and jumps to err_free_area,
free_vm_area(area) calls remove_vm_area() -> free_unmap_vmap_area() ->
vunmap_range_noflush() -> kmsan_vunmap_range_noflush(), which unmaps the
shadow and origin PTEs without freeing the underlying pages.

In contrast, iounmap() in arch/x86/mm/ioremap.c explicitly calls
kmsan_iounmap_page_range() before remove_vm_area() so KMSAN can look up and
free the shadow and origin pages before their PTEs are cleared:

arch/x86/mm/ioremap.c:iounmap() {
	...
	kmsan_iounmap_page_range((unsigned long)addr,
		(unsigned long)addr + get_vm_area_size(p));
	memtype_free(p->phys_addr, p->phys_addr + get_vm_area_size(p));

	/* Finally remove it */
	o = remove_vm_area((void __force *)addr);
	...
}

Should the failure path after ioremap_page_range() call
kmsan_iounmap_page_range(vaddr, vaddr + size) before free_vm_area(area)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031003309.3751-1-jason.andryuk@amd.com?part=1

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

* Re: [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV
  2026-10-31  0:33 [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV Jason Andryuk
  2026-10-31  0:45 ` sashiko-bot
@ 2026-10-31  3:20 ` Borislav Petkov
  1 sibling, 0 replies; 3+ messages in thread
From: Borislav Petkov @ 2026-10-31  3:20 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Juergen Gross, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, x86, H. Peter Anvin, Andy Lutomirski,
	Peter Zijlstra, Boris Ostrovsky, Yazen Ghannam, Mario Limonciello,
	Sean Christopherson, Ingo Molnar, virtualization, linux-kernel,
	xen-devel

On Fri, Oct 30, 2026 at 08:33:08PM -0400, Jason Andryuk wrote:
> Xen PV guests update page tables via hypercalls, and Xen refuses to let a
> domain map machine frames it does not own, such as chipset MMIO in a domU.
> The set_pte() paths ignore the hypercall result, so ioremap() returns a
> mapping whose PTEs are not present.  Accessing the virtual address then
> faults.  This was seen in print_s5_reset_status_mmio() on Xen PV domUs
> on AMD Zen hosts.  The machine address is not accessible by the domU and
> faults on access.
> 
> Add a pv_mmu_ops hook, ioremap_verify_range, which __ioremap_caller()
> calls after ioremap_page_range().  It defaults to returning success.
> Xen PV implements it by walking the range with lookup_address() and
> failing if any entry is not present, making ioremap() return NULL.
> 
> This also affects a Xen PV dom0: an ioremap() of a range containing any
> page Xen refuses to map (e.g. MMIO Xen reserves for itself) now fails as
> a whole, rather than succeeding and faulting only when the refused page
> is accessed.  Pages Xen forces read-only are still mapped present and
> are unaffected.
> 
> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")

That patch is an innocent bystander, if anything - nothing it can do about Xen
being weird about not mapping host physical addresses not owned by the domain.

And TBH, all that gymnastics just to prevent this from running on Xen, is not
something that makes me all happy and so on.

What is more, we try to get rid of PV ops, not add more.

I'm sorry if I've misled you with my comment on the previous thread - I didn't
think someone would even act on it.

And I know Sean doesn't like X86_FEATURE_HYPERVISOR but sorry, nothing beats
a oneliner at the beginning of print_s5_reset_status_mmio() vs this overkill.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette


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

end of thread, other threads:[~2026-10-31  3:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-31  0:33 [PATCH] x86/paravirt: Add ioremap_verify_range hook and implement it for Xen PV Jason Andryuk
2026-10-31  0:45 ` sashiko-bot
2026-10-31  3:20 ` Borislav Petkov

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.