mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "David Hildenbrand (Arm)" <david@kernel.org>
To: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>,
	akpm@linux-foundation.org
Cc: ljs@kernel.org, liam@infradead.org, vbabka@kernel.org,
	rppt@kernel.org, surenb@google.com, mhocko@suse.com,
	peterx@redhat.com, dave.hansen@linux.intel.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	syzbot+49b1021becba70c1f3f6@syzkaller.appspotmail.com
Subject: Re: [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys()
Date: Thu, 1 Oct 2026 22:22:29 +0200	[thread overview]
Message-ID: <32fe40e2-854a-47ec-9d95-93e23f9a7af4@kernel.org> (raw)
In-Reply-To: <20261001152524.171115-1-ngocthang2710.1999@gmail.com>

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

  reply	other threads:[~2026-10-01 20:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-22 11:33 [syzbot] [kernel?] WARNING in __ioremap_caller syzbot
2026-09-04 22:02 ` syzbot
2026-09-04 23:20   ` Dave Hansen
2026-10-01 15:25 ` [PATCH] mm: don't ioremap COWed anon pages in generic_access_phys() Nguyen Ngoc Thang
2026-10-01 20:22   ` David Hildenbrand (Arm) [this message]
2026-10-02  8:38     ` Lorenzo Stoakes (ARM)
2026-10-02 10:02       ` David Hildenbrand (Arm)
2026-10-02 10:04         ` David Hildenbrand (Arm)
2026-10-02 10:09           ` Lorenzo Stoakes (ARM)
2026-10-02 12:19             ` David Hildenbrand (Arm)
2026-10-02 14:03               ` Lorenzo Stoakes (ARM)
2026-10-02 14:26                 ` David Hildenbrand (Arm)
2026-10-02 10:07         ` Lorenzo Stoakes (ARM)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32fe40e2-854a-47ec-9d95-93e23f9a7af4@kernel.org \
    --to=david@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=ngocthang2710.1999@gmail.com \
    --cc=peterx@redhat.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=syzbot+49b1021becba70c1f3f6@syzkaller.appspotmail.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®