[PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[Nguyen Ngoc Thang]

A MAP_PRIVATE mapping of iomem (e.g. a PCI sysfs resourceN file) is a
COW pfnmap: a write fault replaces the pfn with an anonymous page, which
remap_pfn_range() allows by keeping vm_pgoff equal to the base pfn.

generic_access_phys() does not tell those COWed pages apart from the
original pfns and ioremaps whatever the PTE points to. Reading such an
address via /proc/pid/mem or ptrace then ioremaps RAM:

  ioremap on RAM at 0x0000000045623000 - 0x0000000045623fff
  WARNING: arch/x86/mm/ioremap.c:216 at __ioremap_caller.isra.0+0x4c2/0x5f0
  Call Trace:
   generic_access_phys+0x130/0x4d0 mm/memory.c:7178
   kernfs_vma_access+0x1ce/0x280 fs/kernfs/file.c:437
   __access_remote_vm+0x58f/0x890 mm/memory.c:7256
   mem_rw+0x2a1/0x670 fs/proc/base.c:912

Reject COWed pages using the same linearity rule vm_normal_page() uses.
All generic_access_phys() users set up the mapping with a full-vma
(io_)remap_pfn_range(), so the rule holds for them. The access already
fails on x86 and arm64, whose ioremap refuses RAM; elsewhere it no longer
reads the anon page through a device-memory alias.

Fixes: 28b2ee20c7cb ("access_process_vm device memory infrastructure")
Reported-by: syzbot+49b1021becba70c1f3f6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=49b1021becba70c1f3f6
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
This answers Dave's question on the syzbot thread of how a PCI BAR can
point at RAM: it doesn't. The repro mmap()s resource1 MAP_PRIVATE with
PROT_WRITE at address 0, then (via syz_ublk_add_dev() with NULL args)
writes to address 0. That write COWs the BAR page into an anonymous
page, and the later /proc/self/mem read hands its pfn to ioremap.

Forbidding MAP_PRIVATE on BARs would not cover /dev/mem, uio, cdx or
dfl-afu, which share this ->access hook, and private pfnmap COW is
deliberately supported (see get_remap_pgoff()).

Tested in QEMU (q35, virtio-net at 00:03.0) with a small program that
maps resource1 shared and private, COWs the private page and reads both
via pread(/proc/self/mem):

                  before              after
  shared          4, 0xfee01000       4, 0xfee01000
  private         4, 0xfee01000       4, 0xfee01000
  private-cowed   -1 EIO + WARN       -1 EIO, no WARN

(pread() sees EIO either way; internally generic_access_phys() went from
ioremap WARN + -ENOMEM to -EINVAL.)

plus 4 x 500 parallel runs on the patched kernel, no warnings.

Note: the syzbot report also groups an unrelated signature under the
same title (pcibios_device_add() -> memremap() of setup_data on PCI
rescan); this patch does not address that one.

 mm/memory.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 8b0c2c735d3d..8551b144028d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7141,6 +7141,14 @@ void follow_pfnmap_end(struct follow_pfnmap_args *args)
 EXPORT_SYMBOL_GPL(follow_pfnmap_end);
 
 #ifdef CONFIG_HAVE_IOREMAP_PROT
+/* A write fault on a private pfnmap replaces the pfn with an anon page. */
+static bool pfnmap_pfn_is_cowed(struct vm_area_struct *vma, unsigned long addr,
+				unsigned long pfn)
+{
+	return (vma->vm_flags & VM_PFNMAP) && vma_is_cow_mapping(vma) &&
+	       pfn != linear_page_index(vma, addr);
+}
+
 /**
  * generic_access_phys - generic implementation for iomem mmap access
  * @vma: the vma to access
@@ -7175,6 +7183,9 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	if ((write & FOLL_WRITE) && !writable)
 		return -EINVAL;
 
+	if (pfnmap_pfn_is_cowed(vma, addr, phys_addr >> PAGE_SHIFT))
+		return -EINVAL;
+
 	maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
 	if (!maddr)
 		return -ENOMEM;
-- 
2.43.0

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[David Hildenbrand (Arm)]

On 10/1/26 17:25, Nguyen Ngoc Thang wrote:
> A MAP_PRIVATE mapping of iomem (e.g. a PCI sysfs resourceN file) is a
> COW pfnmap: a write fault replaces the pfn with an anonymous page, which
> remap_pfn_range() allows by keeping vm_pgoff equal to the base pfn.
> 
> generic_access_phys() does not tell those COWed pages apart from the
> original pfns and ioremaps whatever the PTE points to. Reading such an
> address via /proc/pid/mem or ptrace then ioremaps RAM:
> 
>   ioremap on RAM at 0x0000000045623000 - 0x0000000045623fff
>   WARNING: arch/x86/mm/ioremap.c:216 at __ioremap_caller.isra.0+0x4c2/0x5f0
>   Call Trace:
>    generic_access_phys+0x130/0x4d0 mm/memory.c:7178
>    kernfs_vma_access+0x1ce/0x280 fs/kernfs/file.c:437
>    __access_remote_vm+0x58f/0x890 mm/memory.c:7256
>    mem_rw+0x2a1/0x670 fs/proc/base.c:912
> 

Ack, apart from that, nothing bad should happen.

> Reject COWed pages using the same linearity rule vm_normal_page() uses.

Hm I don't enjoy that.

What about letting follow_pfnmap_start() just perform a vm_normal_page()
etc and indicate that information to the caller in the
follow_pfnmap_args() ?

We kind-of have that information in the form of follow_pfnmap_args.special
... but it's not available on all architectures. And it shouldn't exist.

I was thinking a while ago about disallowing follow_pfnmap_start() entirely
on anon folios, but the VM_IO thingy made me assume that there are some
odd users (kvm/vfio) that actually need CoWed folios.

Anyhow, this is what I think we should do for now:


From 0b75f30b137612f18cdee28c8c0dfa96870868c2 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Thu, 1 Oct 2026 22:19:57 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 include/linux/mm.h |  4 +++-
 mm/memory.c        | 48 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index c49ef99b4413..b90e547797a9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3193,6 +3193,8 @@ struct folio *vm_normal_folio_pmd(struct vm_area_struct *vma,
 				  unsigned long addr, pmd_t pmd);
 struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
 				pmd_t pmd);
+struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
+		unsigned long addr, pud_t pud);
 struct page *vm_normal_page_pud(struct vm_area_struct *vma, unsigned long addr,
 		pud_t pud);
 
@@ -3245,7 +3247,7 @@ struct follow_pfnmap_args {
 	unsigned long addr_mask;
 	pgprot_t pgprot;
 	bool writable;
-	bool special;
+	bool cow;
 };
 int follow_pfnmap_start(struct follow_pfnmap_args *args);
 void follow_pfnmap_end(struct follow_pfnmap_args *args);
diff --git a/mm/memory.c b/mm/memory.c
index 926276d41920..764b79204ab7 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -843,7 +843,7 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
  * @addr: The address where the @pmd is mapped.
  * @pmd: The PMD.
  *
- * Get the "struct folio" associated with a PTE. See __vm_normal_page()
+ * Get the "struct folio" associated with a PMD. See __vm_normal_page()
  * for details on "normal" and "special" mappings.
  *
  * Return: Returns the "struct folio" if this is a "normal" mapping. Returns
@@ -879,6 +879,28 @@ struct page *vm_normal_page_pud(struct vm_area_struct *vma,
 	return __vm_normal_page(vma, addr, pud_pfn(pud), pud_special(pud),
 				&entry, sizeof(entry), PGTABLE_LEVEL_PUD);
 }
+
+/**
+ * vm_normal_folio_pud() - Get the "struct folio" associated with a PUD
+ * @vma: The VMA mapping the @pud.
+ * @addr: The address where the @pud is mapped.
+ * @pud: The PUD.
+ *
+ * Get the "struct folio" associated with a PUD. See __vm_normal_page()
+ * for details on "normal" and "special" mappings.
+ *
+ * Return: Returns the "struct folio" if this is a "normal" mapping. Returns
+ *	   NULL if this is a "special" mapping.
+ */
+struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
+				  unsigned long addr, pud_t pud)
+{
+	struct page *page = vm_normal_page_pud(vma, addr, pud);
+
+	if (page)
+		return page_folio(page);
+	return NULL;
+}
 #endif
 
 /**
@@ -6947,7 +6969,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
 				     spinlock_t *lock, pte_t *ptep,
 				     pgprot_t pgprot, unsigned long pfn_base,
 				     unsigned long addr_mask, bool writable,
-				     bool special)
+				     struct folio *folio)
 {
 	args->lock = lock;
 	args->ptep = ptep;
@@ -6955,7 +6977,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
 	args->addr_mask = addr_mask;
 	args->pgprot = pgprot;
 	args->writable = writable;
-	args->special = special;
+	args->cow = folio && folio_test_anon(folio);
 }
 
 static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
@@ -7008,6 +7030,7 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
 	struct vm_area_struct *vma = args->vma;
 	unsigned long address = args->address;
 	struct mm_struct *mm = vma->vm_mm;
+	struct folio *folio = NULL;
 	spinlock_t *lock;
 	pgd_t *pgdp;
 	p4d_t *p4dp, p4d;
@@ -7047,9 +7070,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
 			spin_unlock(lock);
 			goto retry;
 		}
+
+		if (vma_is_cow_mapping(vma))
+			folio = vm_normal_folio_pud(vma, address, pud);
 		pfnmap_args_setup(args, lock, NULL, pud_pgprot(pud),
 				  pud_pfn(pud), PUD_MASK, pud_write(pud),
-				  pud_special(pud));
+				  folio);
 		return 0;
 	}
 
@@ -7068,9 +7094,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
 			spin_unlock(lock);
 			goto retry;
 		}
+
+		if (vma_is_cow_mapping(vma))
+			folio = vm_normal_folio_pmd(vma, address, pmd);
 		pfnmap_args_setup(args, lock, NULL, pmd_pgprot(pmd),
 				  pmd_pfn(pmd), PMD_MASK, pmd_write(pmd),
-				  pmd_special(pmd));
+				  folio);
 		return 0;
 	}
 
@@ -7080,9 +7109,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
 	pte = ptep_get(ptep);
 	if (!pte_present(pte))
 		goto unlock;
+
+	if (vma_is_cow_mapping(vma))
+		folio = vm_normal_folio(vma, address, pte);
 	pfnmap_args_setup(args, lock, ptep, pte_pgprot(pte),
 			  pte_pfn(pte), PAGE_MASK, pte_write(pte),
-			  pte_special(pte));
+			  folio);
 	return 0;
 unlock:
 	pte_unmap_unlock(ptep, lock);
@@ -7145,6 +7177,10 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	writable = args.writable;
 	follow_pfnmap_end(&args);
 
+	/* Reject CoW'ed anonymous folios. */
+	if (args.cow)
+		return -EINVAL;
+
 	if ((write & FOLL_WRITE) && !writable)
 		return -EINVAL;
 
-- 
2.43.0


-- 
Cheers,

David

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[Lorenzo Stoakes (ARM)]

On Thu, Oct 01, 2026 at 10:22:29PM +0200, David Hildenbrand (Arm) wrote:
> On 10/1/26 17:25, Nguyen Ngoc Thang wrote:
> > A MAP_PRIVATE mapping of iomem (e.g. a PCI sysfs resourceN file) is a
> > COW pfnmap: a write fault replaces the pfn with an anonymous page, which
> > remap_pfn_range() allows by keeping vm_pgoff equal to the base pfn.
> >
> > generic_access_phys() does not tell those COWed pages apart from the
> > original pfns and ioremaps whatever the PTE points to. Reading such an
> > address via /proc/pid/mem or ptrace then ioremaps RAM:
> >
> >   ioremap on RAM at 0x0000000045623000 - 0x0000000045623fff
> >   WARNING: arch/x86/mm/ioremap.c:216 at __ioremap_caller.isra.0+0x4c2/0x5f0
> >   Call Trace:
> >    generic_access_phys+0x130/0x4d0 mm/memory.c:7178
> >    kernfs_vma_access+0x1ce/0x280 fs/kernfs/file.c:437
> >    __access_remote_vm+0x58f/0x890 mm/memory.c:7256
> >    mem_rw+0x2a1/0x670 fs/proc/base.c:912
> >
>

Sorry to say this looks schlopped.

This guy has sent 10 series across 6 subsystems over ~21 hrs:

https://lore.kernel.org/all/?q=f%3Angocthang2710.1999%40gmail.com

Nguyen - please do not flood the kernel with patches, and please use the
Assisted-by tag for generated content.

The original code you submitted is really not great even if the issue may
be valid.

So I'd say somebody from the core team should take over this if we want to
come up with a patch.

> Ack, apart from that, nothing bad should happen.
>
> > Reject COWed pages using the same linearity rule vm_normal_page() uses.
>
> Hm I don't enjoy that.
>
> What about letting follow_pfnmap_start() just perform a vm_normal_page()
> etc and indicate that information to the caller in the
> follow_pfnmap_args() ?
>
> We kind-of have that information in the form of follow_pfnmap_args.special
> ... but it's not available on all architectures. And it shouldn't exist.
>
> I was thinking a while ago about disallowing follow_pfnmap_start() entirely
> on anon folios, but the VM_IO thingy made me assume that there are some
> odd users (kvm/vfio) that actually need CoWed folios.

Ugh.

>
> Anyhow, this is what I think we should do for now:
>
>
> From 0b75f30b137612f18cdee28c8c0dfa96870868c2 Mon Sep 17 00:00:00 2001
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> Date: Thu, 1 Oct 2026 22:19:57 +0200
> Subject: [PATCH] tmp
>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

This looks reasonable but I hate that we have 'special' CoW overrides like
this :)

> ---
>  include/linux/mm.h |  4 +++-
>  mm/memory.c        | 48 ++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c49ef99b4413..b90e547797a9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3193,6 +3193,8 @@ struct folio *vm_normal_folio_pmd(struct vm_area_struct *vma,
>  				  unsigned long addr, pmd_t pmd);
>  struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
>  				pmd_t pmd);
> +struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
> +		unsigned long addr, pud_t pud);

Hmm, if not defined before why would this need a new PUD handler? Do we
even have PUD-leaf PFN mappings?

>  struct page *vm_normal_page_pud(struct vm_area_struct *vma, unsigned long addr,
>  		pud_t pud);
>
> @@ -3245,7 +3247,7 @@ struct follow_pfnmap_args {
>  	unsigned long addr_mask;
>  	pgprot_t pgprot;
>  	bool writable;
> -	bool special;
> +	bool cow;
>  };
>  int follow_pfnmap_start(struct follow_pfnmap_args *args);
>  void follow_pfnmap_end(struct follow_pfnmap_args *args);
> diff --git a/mm/memory.c b/mm/memory.c
> index 926276d41920..764b79204ab7 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -843,7 +843,7 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
>   * @addr: The address where the @pmd is mapped.
>   * @pmd: The PMD.
>   *
> - * Get the "struct folio" associated with a PTE. See __vm_normal_page()
> + * Get the "struct folio" associated with a PMD. See __vm_normal_page()
>   * for details on "normal" and "special" mappings.
>   *
>   * Return: Returns the "struct folio" if this is a "normal" mapping. Returns
> @@ -879,6 +879,28 @@ struct page *vm_normal_page_pud(struct vm_area_struct *vma,
>  	return __vm_normal_page(vma, addr, pud_pfn(pud), pud_special(pud),
>  				&entry, sizeof(entry), PGTABLE_LEVEL_PUD);
>  }
> +
> +/**
> + * vm_normal_folio_pud() - Get the "struct folio" associated with a PUD
> + * @vma: The VMA mapping the @pud.
> + * @addr: The address where the @pud is mapped.
> + * @pud: The PUD.
> + *
> + * Get the "struct folio" associated with a PUD. See __vm_normal_page()
> + * for details on "normal" and "special" mappings.
> + *
> + * Return: Returns the "struct folio" if this is a "normal" mapping. Returns
> + *	   NULL if this is a "special" mapping.
> + */
> +struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
> +				  unsigned long addr, pud_t pud)
> +{
> +	struct page *page = vm_normal_page_pud(vma, addr, pud);
> +
> +	if (page)
> +		return page_folio(page);
> +	return NULL;
> +}
>  #endif
>
>  /**
> @@ -6947,7 +6969,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
>  				     spinlock_t *lock, pte_t *ptep,
>  				     pgprot_t pgprot, unsigned long pfn_base,
>  				     unsigned long addr_mask, bool writable,
> -				     bool special)
> +				     struct folio *folio)
>  {
>  	args->lock = lock;
>  	args->ptep = ptep;
> @@ -6955,7 +6977,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
>  	args->addr_mask = addr_mask;
>  	args->pgprot = pgprot;
>  	args->writable = writable;
> -	args->special = special;
> +	args->cow = folio && folio_test_anon(folio);

I actually wonder if this should be changed to folio_has_anon_rmap() at
some point :)

Since a folio being 'anon' is vague, because we stupidly made 'anon' vague
in general.

Anyway I was going to ask does this suffice for CoW but having an anon rmap
implies CoW so it does.

>  }
>
>  static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
> @@ -7008,6 +7030,7 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
>  	struct vm_area_struct *vma = args->vma;
>  	unsigned long address = args->address;
>  	struct mm_struct *mm = vma->vm_mm;
> +	struct folio *folio = NULL;
>  	spinlock_t *lock;
>  	pgd_t *pgdp;
>  	p4d_t *p4dp, p4d;
> @@ -7047,9 +7070,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
>  			spin_unlock(lock);
>  			goto retry;
>  		}
> +
> +		if (vma_is_cow_mapping(vma))
> +			folio = vm_normal_folio_pud(vma, address, pud);
>  		pfnmap_args_setup(args, lock, NULL, pud_pgprot(pud),
>  				  pud_pfn(pud), PUD_MASK, pud_write(pud),
> -				  pud_special(pud));
> +				  folio);
>  		return 0;
>  	}
>
> @@ -7068,9 +7094,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
>  			spin_unlock(lock);
>  			goto retry;
>  		}
> +
> +		if (vma_is_cow_mapping(vma))
> +			folio = vm_normal_folio_pmd(vma, address, pmd);
>  		pfnmap_args_setup(args, lock, NULL, pmd_pgprot(pmd),
>  				  pmd_pfn(pmd), PMD_MASK, pmd_write(pmd),
> -				  pmd_special(pmd));
> +				  folio);
>  		return 0;
>  	}
>
> @@ -7080,9 +7109,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
>  	pte = ptep_get(ptep);
>  	if (!pte_present(pte))
>  		goto unlock;
> +
> +	if (vma_is_cow_mapping(vma))
> +		folio = vm_normal_folio(vma, address, pte);
>  	pfnmap_args_setup(args, lock, ptep, pte_pgprot(pte),
>  			  pte_pfn(pte), PAGE_MASK, pte_write(pte),
> -			  pte_special(pte));
> +			  folio);

This all seems reasonable, I guess, insofar as the horrible 'special' CoW
handling is reasonable, but this function is gross to have this degree of
duplication across page table levels.

But I guess one for the future.

>  	return 0;
>  unlock:
>  	pte_unmap_unlock(ptep, lock);
> @@ -7145,6 +7177,10 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  	writable = args.writable;
>  	follow_pfnmap_end(&args);
>
> +	/* Reject CoW'ed anonymous folios. */
> +	if (args.cow)
> +		return -EINVAL;
> +
>  	if ((write & FOLL_WRITE) && !writable)
>  		return -EINVAL;
>
> --
> 2.43.0
>
>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[David Hildenbrand (Arm)]

On 10/2/26 10:38, Lorenzo Stoakes (ARM) wrote:
> On Thu, Oct 01, 2026 at 10:22:29PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/1/26 17:25, Nguyen Ngoc Thang wrote:
>>> A MAP_PRIVATE mapping of iomem (e.g. a PCI sysfs resourceN file) is a
>>> COW pfnmap: a write fault replaces the pfn with an anonymous page, which
>>> remap_pfn_range() allows by keeping vm_pgoff equal to the base pfn.
>>>
>>> generic_access_phys() does not tell those COWed pages apart from the
>>> original pfns and ioremaps whatever the PTE points to. Reading such an
>>> address via /proc/pid/mem or ptrace then ioremaps RAM:
>>>
>>>   ioremap on RAM at 0x0000000045623000 - 0x0000000045623fff
>>>   WARNING: arch/x86/mm/ioremap.c:216 at __ioremap_caller.isra.0+0x4c2/0x5f0
>>>   Call Trace:
>>>    generic_access_phys+0x130/0x4d0 mm/memory.c:7178
>>>    kernfs_vma_access+0x1ce/0x280 fs/kernfs/file.c:437
>>>    __access_remote_vm+0x58f/0x890 mm/memory.c:7256
>>>    mem_rw+0x2a1/0x670 fs/proc/base.c:912
>>>
>>
> 
> Sorry to say this looks schlopped.
> 
> This guy has sent 10 series across 6 subsystems over ~21 hrs:
> 
> https://lore.kernel.org/all/?q=f%3Angocthang2710.1999%40gmail.com
> 
> Nguyen - please do not flood the kernel with patches, and please use the
> Assisted-by tag for generated content.
> 
> The original code you submitted is really not great even if the issue may
> be valid.
> 
> So I'd say somebody from the core team should take over this if we want to
> come up with a patch.

Yes, I'll take care of it.

[...]
>>
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> 
> This looks reasonable but I hate that we have 'special' CoW overrides like
> this :)

After sending this yesterday, I concluded that we can do this cleaner: just have

bool normal_page;

(naming suggestions?)

that express that this is something refcounted with a struct page, like
documented for vm_normal_page().

Then we can just refuse all of these.

> 
>> ---
>>  include/linux/mm.h |  4 +++-
>>  mm/memory.c        | 48 ++++++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 45 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index c49ef99b4413..b90e547797a9 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3193,6 +3193,8 @@ struct folio *vm_normal_folio_pmd(struct vm_area_struct *vma,
>>  				  unsigned long addr, pmd_t pmd);
>>  struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
>>  				pmd_t pmd);
>> +struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
>> +		unsigned long addr, pud_t pud);
> 
> Hmm, if not defined before why would this need a new PUD handler? Do we
> even have PUD-leaf PFN mappings?

Yes we do. In any case, good for consistency.

[...]

>> +	args->cow = folio && folio_test_anon(folio);
> 
> I actually wonder if this should be changed to folio_has_anon_rmap() at
> some point :)

Not a fan.

> 
> Since a folio being 'anon' is vague, because we stupidly made 'anon' vague
> in general.

For folios it's an established term :)

> 
> Anyway I was going to ask does this suffice for CoW but having an anon rmap
> implies CoW so it does.


The downside of using "bool normal_page;" is that we should check
vm_normal_page() for any mapping, not just cow mappings. I suspect
performance-wise we don't really care.

-- 
Cheers,

David

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[David Hildenbrand (Arm)]

On 10/2/26 12:02, David Hildenbrand (Arm) wrote:
> On 10/2/26 10:38, Lorenzo Stoakes (ARM) wrote:
>> On Thu, Oct 01, 2026 at 10:22:29PM +0200, David Hildenbrand (Arm) wrote:
>>>
>>
>> Sorry to say this looks schlopped.
>>
>> This guy has sent 10 series across 6 subsystems over ~21 hrs:
>>
>> https://lore.kernel.org/all/?q=f%3Angocthang2710.1999%40gmail.com
>>
>> Nguyen - please do not flood the kernel with patches, and please use the
>> Assisted-by tag for generated content.
>>
>> The original code you submitted is really not great even if the issue may
>> be valid.
>>
>> So I'd say somebody from the core team should take over this if we want to
>> come up with a patch.
> 
> Yes, I'll take care of it.
> 
> [...]
>>>
>>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>> This looks reasonable but I hate that we have 'special' CoW overrides like
>> this :)
> 
> After sending this yesterday, I concluded that we can do this cleaner: just have
> 
> bool normal_page;
> 
> (naming suggestions?)
> 
> that express that this is something refcounted with a struct page, like
> documented for vm_normal_page().

Hmm, have to think about that once more, regarding VM_IO and if there are some
cases that would actually have to work in generic_access_phys().

-- 
Cheers,

David

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[Lorenzo Stoakes (ARM)]

On Fri, Oct 02, 2026 at 12:04:27PM +0200, David Hildenbrand (Arm) wrote:
> On 10/2/26 12:02, David Hildenbrand (Arm) wrote:
> > On 10/2/26 10:38, Lorenzo Stoakes (ARM) wrote:
> >> On Thu, Oct 01, 2026 at 10:22:29PM +0200, David Hildenbrand (Arm) wrote:
> >>>
> >>
> >> Sorry to say this looks schlopped.
> >>
> >> This guy has sent 10 series across 6 subsystems over ~21 hrs:
> >>
> >> https://lore.kernel.org/all/?q=f%3Angocthang2710.1999%40gmail.com
> >>
> >> Nguyen - please do not flood the kernel with patches, and please use the
> >> Assisted-by tag for generated content.
> >>
> >> The original code you submitted is really not great even if the issue may
> >> be valid.
> >>
> >> So I'd say somebody from the core team should take over this if we want to
> >> come up with a patch.
> >
> > Yes, I'll take care of it.
> >
> > [...]
> >>>
> >>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> >>
> >> This looks reasonable but I hate that we have 'special' CoW overrides like
> >> this :)
> >
> > After sending this yesterday, I concluded that we can do this cleaner: just have
> >
> > bool normal_page;
> >
> > (naming suggestions?)
> >
> > that express that this is something refcounted with a struct page, like
> > documented for vm_normal_page().
>
> Hmm, have to think about that once more, regarding VM_IO and if there are some
> cases that would actually have to work in generic_access_phys().

Well, my series at least makes it easier to reason about VMA_IO_BIT!

Though not sure if it really touches PFN map cases specifically.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[David Hildenbrand (Arm)]

On 10/2/26 12:09, Lorenzo Stoakes (ARM) wrote:
> On Fri, Oct 02, 2026 at 12:04:27PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/2/26 12:02, David Hildenbrand (Arm) wrote:
>>>
>>> Yes, I'll take care of it.
>>>
>>> [...]
>>>
>>> After sending this yesterday, I concluded that we can do this cleaner: just have
>>>
>>> bool normal_page;
>>>
>>> (naming suggestions?)
>>>
>>> that express that this is something refcounted with a struct page, like
>>> documented for vm_normal_page().
>>
>> Hmm, have to think about that once more, regarding VM_IO and if there are some
>> cases that would actually have to work in generic_access_phys().
> 
> Well, my series at least makes it easier to reason about VMA_IO_BIT!
> 
> Though not sure if it really touches PFN map cases specifically.

I'm more concerned about someone using this function on VM_MIXEDMAP | VM_IO with
a memory page that has a struct page but is actually not memory. So we could get
something that vm_normal_page() would flag but generic_access_phys() could
actually read ... I'll have to explore the generic_access_phys() users once more.

All way to complicated (and you series improves things).

-- 
Cheers,

David

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[Lorenzo Stoakes (ARM)]

On Fri, Oct 02, 2026 at 02:19:00PM +0200, David Hildenbrand (Arm) wrote:
> On 10/2/26 12:09, Lorenzo Stoakes (ARM) wrote:
> > On Fri, Oct 02, 2026 at 12:04:27PM +0200, David Hildenbrand (Arm) wrote:
> >> On 10/2/26 12:02, David Hildenbrand (Arm) wrote:
> >>>
> >>> Yes, I'll take care of it.
> >>>
> >>> [...]
> >>>
> >>> After sending this yesterday, I concluded that we can do this cleaner: just have
> >>>
> >>> bool normal_page;
> >>>
> >>> (naming suggestions?)
> >>>
> >>> that express that this is something refcounted with a struct page, like
> >>> documented for vm_normal_page().
> >>
> >> Hmm, have to think about that once more, regarding VM_IO and if there are some
> >> cases that would actually have to work in generic_access_phys().
> >
> > Well, my series at least makes it easier to reason about VMA_IO_BIT!
> >
> > Though not sure if it really touches PFN map cases specifically.
>
> I'm more concerned about someone using this function on VM_MIXEDMAP | VM_IO with
> a memory page that has a struct page but is actually not memory. So we could get
> something that vm_normal_page() would flag but generic_access_phys() could
> actually read ... I'll have to explore the generic_access_phys() users once more.

Hmm that could be a problem also for struct page's that are there but you're not
supposed to access for other reasons to as well?

Definitely need to be careful about that.

>
> All way to complicated (and you series improves things).

:) well there's still a lot of complexity in there, one battle at a time...

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[David Hildenbrand (Arm)]

On 10/2/26 16:03, Lorenzo Stoakes (ARM) wrote:
> On Fri, Oct 02, 2026 at 02:19:00PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/2/26 12:09, Lorenzo Stoakes (ARM) wrote:
>>>
>>> Well, my series at least makes it easier to reason about VMA_IO_BIT!
>>>
>>> Though not sure if it really touches PFN map cases specifically.
>>
>> I'm more concerned about someone using this function on VM_MIXEDMAP | VM_IO with
>> a memory page that has a struct page but is actually not memory. So we could get
>> something that vm_normal_page() would flag but generic_access_phys() could
>> actually read ... I'll have to explore the generic_access_phys() users once more.
> 
> Hmm that could be a problem also for struct page's that are there but you're not
> supposed to access for other reasons to as well?
> 
> Definitely need to be careful about that.
> 
>>
>> All way to complicated (and you series improves things).
> 
> :) well there's still a lot of complexity in there, one battle at a time...

My conclusion so far is: indicating is_normal should work when using it in
generic_access_phys(), as it is never used on non-VM_PFNMAP VMAs.

follow_pfnmap_start(), however, can be called on some non-VM_PFNMAP-but-VM_IO
VMAs from KVM as it seems.

So we cannot easily restrict follow_pfnmap_start() to VM_PFNMAP only (weird,
right?) without risking breaking some weird KVM use case. Gah.

Anyhow, I'll prepare a fix for this one and send it out likely later today.

-- 
Cheers,

David

Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()

[Lorenzo Stoakes (ARM)]

On Fri, Oct 02, 2026 at 12:02:15PM +0200, David Hildenbrand (Arm) wrote:
> On 10/2/26 10:38, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Oct 01, 2026 at 10:22:29PM +0200, David Hildenbrand (Arm) wrote:

> > So I'd say somebody from the core team should take over this if we want to
> > come up with a patch.
>
> Yes, I'll take care of it.

Thanks!

> >> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> >
> > This looks reasonable but I hate that we have 'special' CoW overrides like
> > this :)
>
> After sending this yesterday, I concluded that we can do this cleaner: just have
>
> bool normal_page;
>
> (naming suggestions?)
>
> that express that this is something refcounted with a struct page, like
> documented for vm_normal_page().
>
> Then we can just refuse all of these.

Yeah it's all a bit tricky.

Maybe is_vm_normal_page ?

Or do we want to default to referring to a folio... but then pfnmap not a
folio... ugh.

is_normal?

With a comment explaining in sense of vm_normal_page/folio()?

> >> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >> index c49ef99b4413..b90e547797a9 100644
> >> --- a/include/linux/mm.h
> >> +++ b/include/linux/mm.h

> >>  				  unsigned long addr, pmd_t pmd);
> >>  struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
> >>  				pmd_t pmd);
> >> +struct folio *vm_normal_folio_pud(struct vm_area_struct *vma,
> >> +		unsigned long addr, pud_t pud);
> >
> > Hmm, if not defined before why would this need a new PUD handler? Do we
> > even have PUD-leaf PFN mappings?
>
> Yes we do. In any case, good for consistency.

In that case then we should definitely have it!

> > Since a folio being 'anon' is vague, because we stupidly made 'anon' vague
> > in general.
>
> For folios it's an established term :)

Yeah, fair enough, that is true.

And makes the anon folio -> tracked as such in my change consistent end-to-end.

I guess we have swapbacked for the shmem stuff as a clear delineation too.

>
> >
> > Anyway I was going to ask does this suffice for CoW but having an anon rmap
> > implies CoW so it does.
>
>
> The downside of using "bool normal_page;" is that we should check
> vm_normal_page() for any mapping, not just cow mappings. I suspect
> performance-wise we don't really care.

Yep, I'm sure it's fine!

--
Cheers, Lorenzo