The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock
@ 2026-07-17 17:00 Rik van Riel
  2026-07-17 17:00 ` [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked() Rik van Riel
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm

__access_remote_vm() holds mmap_read_lock() for the whole transfer. On
large machines with large multi-threaded applications, the mmap_lock
is often contended due to mixed accesses from readers and writers, like
mmap and munmap. When a lock holder gets stuck, system monitoring software
can get stuck behind that, resulting in a failure to log that the system
is in trouble.

Take the per-VMA lock in __access_remote_vm() when the access falls
entirely within a single VMA. Fall back to the mmap lock when the access
crosses a VMA boundary, or when the page cannot be reached under the
per-VMA lock: a dropped fault, a userfaultfd VMA, a hard error, or memory
with no struct page that has to go through vma->vm_ops->access().

The bulk of the work is a new gup helper. __access_remote_vm() needs a
single page from a VMA it has already looked up and locked, faulting it in
when necessary, under either lock.

get_user_pages_remote() does not fit: it hard codes the mmap lock and
re-derives the VMA. get_user_page_vma() walks the page tables, faults a
missing page in, and returns it with a reference and the caller's lock
still held.

The per-VMA path also closes a pre-existing gap. A COWed page in a
VM_IO/VM_PFNMAP VMA has a struct page, but the old code routed it to
->access(), where generic_access_phys() ioremaps the PFN and ioremap of RAM
is rejected, so the read came up short.

get_user_page_vma() now returns that page normally. Raw PFNs with no struct
page still reach ->access() under the mmap lock, as before.

The series is arranged as:

  1-2: untag the remote address in the VMA lookup without the mmap lock,
       on x86 and riscv.
  3:   rename get_user_page_vma_remote() to get_user_page_lookup_vma().
  4:   add get_user_page_vma().
  5:   switch __access_remote_vm() to the per-VMA lock.
  6:   add selftest coverage.

Changes since v2 [1]. The per-VMA fast path is reworked to build on the GUP
lookup+fault path instead of a custom walker, per David Hildenbrand's
and Lorenzo Stoakes' review, and now faults pages in rather than only
reading resident ones.

  - Drop the folio_walk_start(FW_VMA_LOCKED) walk. Add get_user_page_vma()
    in mm/gup.c (patch 4): a trimmed __get_user_pages() that walks with
    follow_page_mask(), faults in with faultin_page(), and returns the page
    with the caller's lock held. Safe under the per-VMA lock because page
    tables are RCU-freed, so interrupts need not be disabled. (David)
  - Fault pages in on the fast path. v2 fell back to the mmap lock for any
    not-present page; now it falls back only on -EAGAIN. (David)
  - Turn the v2 READ_ONCE/WRITE_ONCE untag_mask change into an
    untagged_addr_remote_unlocked() helper (patch 1), and add the riscv
    pointer-masking equivalent (patch 2), which v2 did not cover.
  - Read COWed pages in VM_IO/VM_PFNMAP VMAs; raw PFNs still fall back to
    ->access() under the mmap lock.
  - Add the get_user_page_lookup_vma() rename (patch 3) and selftest
    coverage for the struct-page and raw-PFN paths (patch 6).

  [1] https://lore.kernel.org/all/20260625015053.2445008-1-riel@surriel.com/

Rik van Riel (6):
      x86/mm: add untagged_addr_remote_unlocked()
      riscv/mm: add untagged_addr_remote_unlocked()
      mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma()
      mm/gup: add get_user_page_vma() to fault in a page under a held lock
      mm: use per-VMA lock in __access_remote_vm() for single-VMA accesses
      selftests/mm: cover /proc/pid/mem access to VM_PFNMAP memory

 arch/arm64/kernel/mte.c              |   2 +-
 arch/riscv/include/asm/mmu_context.h |   4 +-
 arch/riscv/include/asm/uaccess.h     |  10 +-
 arch/riscv/kernel/process.c          |  12 +-
 arch/x86/include/asm/mmu_context.h   |   6 +-
 arch/x86/include/asm/uaccess_64.h    |  14 ++-
 arch/x86/kernel/process_64.c         |   4 +-
 arch/x86/kernel/uprobes.c            |   2 +-
 include/linux/mm.h                   |   2 +-
 include/linux/uaccess.h              |   7 ++
 mm/gup.c                             | 156 ++++++++++++++++++++++----
 mm/internal.h                        |   7 +-
 mm/memory.c                          | 171 ++++++++++++++++++++-------
 mm/rmap.c                            |   2 +-
 tools/testing/selftests/mm/pfnmap.c  |  66 +++++++++++
 15 files changed, 375 insertions(+), 90 deletions(-)

base-commit: 0f26556c5eeea62cc934fa8938b148aa5844a6b6
--
2.47.0

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

* [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked()
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-20 11:12   ` Usama Arif
  2026-07-17 17:00 ` [PATCH RFC v3 2/6] riscv/mm: " Rik van Riel
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Usama Arif

__access_remote_vm() reads another process's memory under the mmap lock. On
large machines that lock is contended by tasks polling /proc/PID/cmdline,
/proc/PID/environ, or calling process_vm_readv(), even though the memory
they read is almost always resident and could be reached under the per-VMA
lock instead.

Looking up the VMA first requires untagging the address.
untagged_addr_remote() asserts the mmap lock only because it reads
mm->context.untag_mask, which can race with the write in mm_enable_lam().

That mask is set once, when LAM is enabled, and never changes afterwards,
so the read itself does not need the lock. It is already read without it,
from the context switch path and /proc/PID/status.

Add untagged_addr_remote_unlocked() for callers that have not taken the
mmap lock, and annotate access to mm->context.untag_mask with READ_ONCE()
and WRITE_ONCE() so the existing lockless reads are explicit and
KCSAN-clean. untagged_addr_remote() keeps its assertion and shares the same
code.

Assisted-by: Claude:claude-opus-4.8
Acked-by: Usama Arif <usamaarif642@gmail.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 arch/x86/include/asm/mmu_context.h |  6 +++---
 arch/x86/include/asm/uaccess_64.h  | 14 +++++++++++---
 arch/x86/kernel/process_64.c       |  4 ++--
 include/linux/uaccess.h            |  7 +++++++
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index ef5b507de34e..cee710f64658 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -100,18 +100,18 @@ static inline unsigned long mm_lam_cr3_mask(struct mm_struct *mm)
 static inline void dup_lam(struct mm_struct *oldmm, struct mm_struct *mm)
 {
 	mm->context.lam_cr3_mask = oldmm->context.lam_cr3_mask;
-	mm->context.untag_mask = oldmm->context.untag_mask;
+	WRITE_ONCE(mm->context.untag_mask, READ_ONCE(oldmm->context.untag_mask));
 }
 
 #define mm_untag_mask mm_untag_mask
 static inline unsigned long mm_untag_mask(struct mm_struct *mm)
 {
-	return mm->context.untag_mask;
+	return READ_ONCE(mm->context.untag_mask);
 }
 
 static inline void mm_reset_untag_mask(struct mm_struct *mm)
 {
-	mm->context.untag_mask = -1UL;
+	WRITE_ONCE(mm->context.untag_mask, -1UL);
 }
 
 #define arch_pgtable_dma_compat arch_pgtable_dma_compat
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index 20de34cc9aa6..05377ad804d1 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -39,18 +39,26 @@ static inline unsigned long __untagged_addr(unsigned long addr)
 	(__force __typeof__(addr))__untagged_addr(__addr);		\
 })
 
+/*
+ * mm->context.untag_mask is set once, when LAM is enabled, and never
+ * changes afterwards, so it can be read without holding the mmap lock.
+ */
 static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
 						   unsigned long addr)
 {
-	mmap_assert_locked(mm);
-	return addr & (mm)->context.untag_mask;
+	return addr & READ_ONCE(mm->context.untag_mask);
 }
 
-#define untagged_addr_remote(mm, addr)	({				\
+#define untagged_addr_remote_unlocked(mm, addr)	({			\
 	unsigned long __addr = (__force unsigned long)(addr);		\
 	(__force __typeof__(addr))__untagged_addr_remote(mm, __addr);	\
 })
 
+#define untagged_addr_remote(mm, addr)	({				\
+	mmap_assert_locked(mm);						\
+	untagged_addr_remote_unlocked(mm, addr);			\
+})
+
 #endif
 
 #define valid_user_address(x) \
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index d44afbe005bb..9fa659117f38 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -814,7 +814,7 @@ static void enable_lam_func(void *__mm)
 static void mm_enable_lam(struct mm_struct *mm)
 {
 	mm->context.lam_cr3_mask = X86_CR3_LAM_U57;
-	mm->context.untag_mask =  ~GENMASK(62, 57);
+	WRITE_ONCE(mm->context.untag_mask, ~GENMASK(62, 57));
 
 	/*
 	 * Even though the process must still be single-threaded at this
@@ -952,7 +952,7 @@ long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2)
 #endif
 #ifdef CONFIG_ADDRESS_MASKING
 	case ARCH_GET_UNTAG_MASK:
-		return put_user(task->mm->context.untag_mask,
+		return put_user(mm_untag_mask(task->mm),
 				(unsigned long __user *)arg2);
 	case ARCH_ENABLE_TAGGED_ADDR:
 		return prctl_enable_tagged_addr(task->mm, arg2);
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index eddbbb65ccc4..7e6e4c89184c 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -34,6 +34,13 @@
 })
 #endif
 
+#ifndef untagged_addr_remote_unlocked
+#define untagged_addr_remote_unlocked(mm, addr)	({	\
+	(void)(mm);					\
+	untagged_addr(addr);				\
+})
+#endif
+
 #ifdef masked_user_access_begin
  #define can_do_masked_user_access() 1
 # ifndef masked_user_write_access_begin
-- 
2.53.0-Meta


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

* [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
  2026-07-17 17:00 ` [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked() Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-20 11:57   ` Usama Arif
  2026-07-17 17:00 ` [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma() Rik van Riel
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv

__access_remote_vm() untags the remote address before looking up the VMA,
now without holding the mmap lock. riscv defines untagged_addr_remote() but
not untagged_addr_remote_unlocked(), so it falls back to the generic
version, which untags with untagged_addr().

That reads current->mm, not the target mm, so a remote access to a process
using pointer masking would untag with the wrong mask.

mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL and is stable
afterwards, so it can be read without the mmap lock, as it already is from
untagged_addr() and mm_untag_mask().

Add untagged_addr_remote_unlocked(), which untags against the target mm,
and annotate context.pmlen accesses with READ_ONCE() and WRITE_ONCE() so
the lockless reads are explicit and KCSAN-clean. untagged_addr_remote()
keeps its mmap_assert_locked() and shares the code.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 arch/riscv/include/asm/mmu_context.h |  4 ++--
 arch/riscv/include/asm/uaccess.h     | 10 +++++++---
 arch/riscv/kernel/process.c          | 12 +++++++-----
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
index dbf27a78df6c..3ce16796e5a2 100644
--- a/arch/riscv/include/asm/mmu_context.h
+++ b/arch/riscv/include/asm/mmu_context.h
@@ -21,7 +21,7 @@ static inline void activate_mm(struct mm_struct *prev,
 			       struct mm_struct *next)
 {
 #ifdef CONFIG_RISCV_ISA_SUPM
-	next->context.pmlen = 0;
+	WRITE_ONCE(next->context.pmlen, 0);
 #endif
 	switch_mm(prev, next, NULL);
 }
@@ -44,7 +44,7 @@ DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
 #define mm_untag_mask mm_untag_mask
 static inline unsigned long mm_untag_mask(struct mm_struct *mm)
 {
-	return -1UL >> mm->context.pmlen;
+	return -1UL >> READ_ONCE(mm->context.pmlen);
 }
 #endif
 
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 5d4ec15584cf..53806e0f7dcf 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -16,7 +16,7 @@
 static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigned long addr)
 {
 	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM)) {
-		u8 pmlen = mm->context.pmlen;
+		u8 pmlen = READ_ONCE(mm->context.pmlen);
 
 		/* Virtual addresses are sign-extended; physical addresses are zero-extended. */
 		if (IS_ENABLED(CONFIG_MMU))
@@ -33,12 +33,16 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
 	(__force __typeof__(addr))__untagged_addr_remote(current->mm, __addr);	\
 })
 
-#define untagged_addr_remote(mm, addr) ({					\
+#define untagged_addr_remote_unlocked(mm, addr) ({				\
 	unsigned long __addr = (__force unsigned long)(addr);			\
-	mmap_assert_locked(mm);							\
 	(__force __typeof__(addr))__untagged_addr_remote(mm, __addr);		\
 })
 
+#define untagged_addr_remote(mm, addr) ({					\
+	mmap_assert_locked(mm);							\
+	untagged_addr_remote_unlocked(mm, addr);				\
+})
+
 #define access_ok(addr, size) likely(__access_ok(untagged_addr(addr), size))
 #else
 #define untagged_addr(addr) (addr)
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index b2df7f72241a..6ae7552fed09 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -357,13 +357,15 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg)
 	if (mmap_write_lock_killable(mm))
 		return -EINTR;
 
-	if (test_bit(MM_CONTEXT_LOCK_PMLEN, &mm->context.flags) && mm->context.pmlen != pmlen) {
-		mmap_write_unlock(mm);
-		return -EBUSY;
+	if (test_bit(MM_CONTEXT_LOCK_PMLEN, &mm->context.flags)) {
+		if (READ_ONCE(mm->context.pmlen) != pmlen) {
+			mmap_write_unlock(mm);
+			return -EBUSY;
+		}
 	}
 
 	envcfg_update_bits(task, ENVCFG_PMM, pmm);
-	mm->context.pmlen = pmlen;
+	WRITE_ONCE(mm->context.pmlen, pmlen);
 
 	mmap_write_unlock(mm);
 
@@ -394,7 +396,7 @@ long get_tagged_addr_ctrl(struct task_struct *task)
 		break;
 	}
 
-	if (task->mm->context.pmlen)
+	if (READ_ONCE(task->mm->context.pmlen))
 		ret |= PR_TAGGED_ADDR_ENABLE;
 
 	return ret;
-- 
2.53.0-Meta


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

* [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma()
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
  2026-07-17 17:00 ` [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked() Rik van Riel
  2026-07-17 17:00 ` [PATCH RFC v3 2/6] riscv/mm: " Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-20 12:00   ` Usama Arif
  2026-07-17 17:00 ` [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock Rik van Riel
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Catalin Marinas,
	Will Deacon, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Harry Yoo, Jann Horn, Lance Yang,
	linux-arm-kernel, linux-trace-kernel

get_user_page_vma_remote() faults in the page at @addr in a remote mm and
also looks up the VMA that covers it, handing both back to the caller.

This cleans up the name space for adding a get_user_page_vma() variant
where the caller already has the vma.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 arch/arm64/kernel/mte.c   | 2 +-
 arch/x86/kernel/uprobes.c | 2 +-
 include/linux/mm.h        | 2 +-
 mm/memory.c               | 4 ++--
 mm/rmap.c                 | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 1a9aad6ef22a..7a6ecc3d9294 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -459,7 +459,7 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,
 		struct vm_area_struct *vma;
 		unsigned long tags, offset;
 		void *maddr;
-		struct page *page = get_user_page_vma_remote(mm, addr,
+		struct page *page = get_user_page_lookup_vma(mm, addr,
 							     gup_flags, &vma);
 		struct folio *folio;
 
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 3af979fb41d3..329efac0cfb3 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -1036,7 +1036,7 @@ static int copy_from_vaddr(struct mm_struct *mm, unsigned long vaddr, void *dst,
 	struct vm_area_struct *vma;
 	struct page *page;
 
-	page = get_user_page_vma_remote(mm, vaddr, gup_flags, &vma);
+	page = get_user_page_lookup_vma(mm, vaddr, gup_flags, &vma);
 	if (IS_ERR(page))
 		return PTR_ERR(page);
 	uprobe_copy_from_page(page, vaddr, dst, len);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 485df9c2dbdd..24ead14b4790 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3238,7 +3238,7 @@ long pin_user_pages_remote(struct mm_struct *mm,
 /*
  * Retrieves a single page alongside its VMA. Does not support FOLL_NOWAIT.
  */
-static inline struct page *get_user_page_vma_remote(struct mm_struct *mm,
+static inline struct page *get_user_page_lookup_vma(struct mm_struct *mm,
 						    unsigned long addr,
 						    int gup_flags,
 						    struct vm_area_struct **vmap)
diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..3b86eeaf084f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7039,7 +7039,7 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
 		void *maddr;
 		struct folio *folio;
 		struct vm_area_struct *vma = NULL;
-		struct page *page = get_user_page_vma_remote(mm, addr,
+		struct page *page = get_user_page_lookup_vma(mm, addr,
 							     gup_flags, &vma);
 
 		if (IS_ERR(page)) {
@@ -7167,7 +7167,7 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
 		struct page *page;
 		struct vm_area_struct *vma = NULL;
 
-		page = get_user_page_vma_remote(mm, addr, gup_flags, &vma);
+		page = get_user_page_lookup_vma(mm, addr, gup_flags, &vma);
 		if (IS_ERR(page)) {
 			/*
 			 * Treat as a total failure for now until we decide how
diff --git a/mm/rmap.c b/mm/rmap.c
index 1c77d5dc06e9..b36f2e219b8f 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -2838,7 +2838,7 @@ struct page *make_device_exclusive(struct mm_struct *mm, unsigned long addr,
 	 * (non-device-exclusive) PTE and issue a MMU_NOTIFY_EXCLUSIVE.
 	 */
 retry:
-	page = get_user_page_vma_remote(mm, addr,
+	page = get_user_page_lookup_vma(mm, addr,
 					FOLL_GET | FOLL_WRITE | FOLL_SPLIT_PMD,
 					&vma);
 	if (IS_ERR(page))
-- 
2.53.0-Meta


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

* [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
                   ` (2 preceding siblings ...)
  2026-07-17 17:00 ` [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma() Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-20 12:35   ` Usama Arif
  2026-07-17 17:00 ` [PATCH RFC v3 5/6] mm: use per-VMA lock in __access_remote_vm() for single-VMA accesses Rik van Riel
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Jason Gunthorpe,
	John Hubbard, Peter Xu

__access_remote_vm() needs a single page from a VMA it has already
looked up and locked, faulting it in when necessary, under either the
mmap lock or the per-VMA lock. get_user_pages_remote() does not fit: it
hard codes the mmap lock and re-looks-up the VMA, neither of which is
wanted here.

Add get_user_page_vma(), a simplified __get_user_pages() that walks the
page tables with follow_page_mask(), faults a missing page in with
faultin_page(), and on success returns it with a reference and the
caller's lock still held. Like __get_user_pages() it runs
check_vma_flags(), so callers need not pre-check the VMA.

A VM_IO/VM_PFNMAP VMA is the exception to that check: it can still hold
COWed pages that have a struct page, so follow_page_mask() is allowed to
look for one. Memory with no struct page -- a raw PFN, or a present PFN
reported as -EEXIST -- is returned as -EFAULT, so the caller can reach it
through vma->vm_ops->access().

The caller sets FOLL_VMA_LOCK when it holds the per-VMA lock rather than
the mmap lock, which reaches the fault code as FAULT_FLAG_VMA_LOCK.

Anything that cannot complete under the per-VMA lock -- a dropped fault,
a userfaultfd VMA (uffd assumes current is the faulting task), a hard
error, or ->access() memory -- releases the lock and returns -EAGAIN,
so the caller retries under the mmap lock.

faultin_page() reports these retries as -EAGAIN for both lock types;
only the mmap caller records the dropped lock in *locked. A
VM_FAULT_ERROR that decodes to no errno warns and returns -EFAULT
rather than BUG(), since the mmap-lock retry produces the definitive
result.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 mm/gup.c      | 154 +++++++++++++++++++++++++++++++++++++++++++-------
 mm/internal.h |   6 +-
 2 files changed, 139 insertions(+), 21 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 0692119b7904..69b834a71708 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1082,7 +1082,13 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address,
 /*
  * mmap_lock must be held on entry.  If @flags has FOLL_UNLOCKABLE but not
  * FOLL_NOWAIT, the mmap_lock may be released.  If it is, *@locked will be set
- * to 0 and -EBUSY returned.
+ * to 0 and -EAGAIN returned.
+ *
+ * The return value does not depend on the lock type: a fault that made
+ * progress but needs a retry (VM_FAULT_RETRY / VM_FAULT_COMPLETED) is reported
+ * as -EAGAIN for both the mmap lock and the per-VMA lock (FOLL_VMA_LOCK). Only
+ * the *@locked side effect is lock-type specific, as the per-VMA lock path has
+ * no unlockable mmap_lock to drop.
  */
 static int faultin_page(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags, bool unshare,
@@ -1097,6 +1103,8 @@ static int faultin_page(struct vm_area_struct *vma,
 		fault_flags |= FAULT_FLAG_WRITE;
 	if (flags & FOLL_REMOTE)
 		fault_flags |= FAULT_FLAG_REMOTE;
+	if (flags & FOLL_VMA_LOCK)
+		fault_flags |= FAULT_FLAG_VMA_LOCK;
 	if (flags & FOLL_UNLOCKABLE) {
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 		/*
@@ -1125,41 +1133,147 @@ static int faultin_page(struct vm_area_struct *vma,
 
 	ret = handle_mm_fault(vma, address, fault_flags, NULL);
 
+	/*
+	 * A fully completed fault (VM_FAULT_COMPLETED) or one that needs a retry
+	 * (VM_FAULT_RETRY) has released the lock it was holding. Report both as
+	 * -EAGAIN so the caller retries: the mmap lock caller retakes it here,
+	 * the per-VMA lock caller (FOLL_VMA_LOCK) falls back to the mmap lock.
+	 *
+	 * Dropping the mmap lock is recorded in *@locked. There is no such lock
+	 * to drop under the per-VMA lock, where @locked is not used, so leave it
+	 * alone in that case.
+	 */
 	if (ret & VM_FAULT_COMPLETED) {
-		/*
-		 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
-		 * mmap lock in the page fault handler. Sanity check this.
-		 */
-		WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
-		*locked = 0;
-
-		/*
-		 * We should do the same as VM_FAULT_RETRY, but let's not
-		 * return -EBUSY since that's not reflecting the reality of
-		 * what has happened - we've just fully completed a page
-		 * fault, with the mmap lock released.  Use -EAGAIN to show
-		 * that we want to take the mmap lock _again_.
-		 */
+		if (!(flags & FOLL_VMA_LOCK)) {
+			/*
+			 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
+			 * mmap lock in the page fault handler. Sanity check this.
+			 */
+			WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
+			*locked = 0;
+		}
 		return -EAGAIN;
 	}
 
 	if (ret & VM_FAULT_ERROR) {
 		int err = vm_fault_to_errno(ret, flags);
 
-		if (err)
-			return err;
-		BUG();
+		/*
+		 * VM_FAULT_ERROR always decodes to an errno; a zero here would
+		 * mean handle_mm_fault() returned an unexpected combination.
+		 * Report -EFAULT rather than crash: under the per-VMA lock the
+		 * mmap lock retry produces the definitive result.
+		 */
+		VM_WARN_ON_ONCE(!err);
+		return err ? err : -EFAULT;
 	}
 
 	if (ret & VM_FAULT_RETRY) {
-		if (!(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
+		if (!(flags & FOLL_VMA_LOCK) &&
+		    !(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
 			*locked = 0;
-		return -EBUSY;
+		return -EAGAIN;
 	}
 
 	return 0;
 }
 
+/*
+ * get_user_page_vma - get one page from @vma, whose lock the caller already
+ * holds: the mmap lock, or (with FOLL_VMA_LOCK) the per-VMA lock. Walks the
+ * page tables, faulting the page in if needed, and on success returns it with
+ * a reference and the lock still held.
+ *
+ * Runs check_vma_flags() like __get_user_pages(), so callers need not pre-check
+ * the VMA; most rejections are returned as their error. A VM_IO/VM_PFNMAP VMA
+ * is the exception: a COWed page with a struct page is returned, while a raw
+ * PFN has none and yields -EFAULT, to be reached via vma->vm_ops->access().
+ *
+ * Under FOLL_VMA_LOCK, anything that cannot be finished under the per-VMA lock
+ * (a dropped fault, userfaultfd, a hard error, or ->access() memory) releases
+ * the lock and returns -EAGAIN, so the caller retries under the mmap lock.
+ */
+struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
+			       unsigned int gup_flags)
+{
+	bool vma_locked = gup_flags & FOLL_VMA_LOCK;
+	unsigned long page_mask;
+	struct page *page;
+	int locked = 1;
+	bool pfnmap;
+	int ret;
+
+	/*
+	 * Validate the VMA up front, like __get_user_pages(). A VM_IO/VM_PFNMAP
+	 * VMA is not rejected outright: it can hold COWed pages that have a
+	 * struct page, so let follow_page_mask() look for one, and treat only
+	 * its struct-page-less PFNs as unreachable. Any other rejection
+	 * (secretmem, bad permissions, ...) is final.
+	 */
+	ret = check_vma_flags(vma, gup_flags);
+	if (ret && !(vma->vm_flags & (VM_IO | VM_PFNMAP)))
+		goto fail;
+	pfnmap = ret;
+
+	for (;;) {
+		if (fatal_signal_pending(current)) {
+			ret = -EINTR;
+			goto fail;
+		}
+		cond_resched();
+
+		page = follow_page_mask(vma, addr,
+					gup_flags | FOLL_TOUCH | FOLL_GET,
+					&page_mask);
+		if (!IS_ERR_OR_NULL(page))
+			return page;
+
+		/*
+		 * No struct page: a raw PFN of a VM_IO/VM_PFNMAP VMA, whether
+		 * seen by the up-front check (@pfnmap) or reported as -EEXIST
+		 * for a present PFN. Return -EFAULT so the caller reaches it
+		 * through vma->vm_ops->access().
+		 */
+		if (pfnmap || PTR_ERR(page) == -EEXIST) {
+			ret = -EFAULT;
+			goto fail;
+		}
+		/* A hard error from the walk itself. */
+		if (page && PTR_ERR(page) != -EMLINK) {
+			ret = PTR_ERR(page);
+			goto fail;
+		}
+
+		/*
+		 * The page is not present, or needs unsharing. A remote fault
+		 * under the per-VMA lock cannot deliver userfaultfd (which
+		 * assumes current is the faulting task), so fall back for those.
+		 */
+		if (vma_locked && userfaultfd_armed(vma)) {
+			ret = -EAGAIN;
+			goto fail;
+		}
+		ret = faultin_page(vma, addr, gup_flags | FOLL_REMOTE | FOLL_GET,
+				   PTR_ERR(page) == -EMLINK, &locked);
+		if (ret == -EAGAIN)
+			return ERR_PTR(-EAGAIN);	/* fault released the per-VMA lock */
+		if (ret)
+			goto fail;
+	}
+
+fail:
+	/*
+	 * Under the per-VMA lock the caller cannot reach ->access() or act on a
+	 * hard error (both need the mmap lock), so release the lock and have it
+	 * retry there; the mmap-lock pass produces the definitive error.
+	 */
+	if (vma_locked) {
+		vma_end_read(vma);
+		return ERR_PTR(-EAGAIN);
+	}
+	return ERR_PTR(ret);
+}
+
 /*
  * Writing to file-backed mappings which require folio dirty tracking using GUP
  * is a fundamentally broken operation, as kernel write access to GUP mappings
diff --git a/mm/internal.h b/mm/internal.h
index 181e79f1d6a2..0899a37907c1 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1595,6 +1595,8 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
  */
 int __must_check try_grab_folio(struct folio *folio, int refs,
 				unsigned int flags);
+struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
+			       unsigned int gup_flags);
 
 /*
  * mm/huge_memory.c
@@ -1641,11 +1643,13 @@ enum {
 	FOLL_UNLOCKABLE = 1 << 21,
 	/* VMA lookup+checks compatible with MADV_POPULATE_(READ|WRITE) */
 	FOLL_MADV_POPULATE = 1 << 22,
+	/* caller holds the per-VMA lock, not the mmap lock */
+	FOLL_VMA_LOCK = 1 << 23,
 };
 
 #define INTERNAL_GUP_FLAGS (FOLL_TOUCH | FOLL_TRIED | FOLL_REMOTE | FOLL_PIN | \
 			    FOLL_FAST_ONLY | FOLL_UNLOCKABLE | \
-			    FOLL_MADV_POPULATE)
+			    FOLL_MADV_POPULATE | FOLL_VMA_LOCK)
 
 /*
  * Indicates for which pages that are write-protected in the page table,
-- 
2.53.0-Meta


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

* [PATCH RFC v3 5/6] mm: use per-VMA lock in __access_remote_vm() for single-VMA accesses
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
                   ` (3 preceding siblings ...)
  2026-07-17 17:00 ` [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-17 17:00 ` [PATCH RFC v3 6/6] selftests/mm: cover /proc/pid/mem access to VM_PFNMAP memory Rik van Riel
  2026-07-21 18:12 ` [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock David Hildenbrand (Arm)
  6 siblings, 0 replies; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Jason Gunthorpe,
	John Hubbard, Peter Xu

__access_remote_vm() holds mmap_read_lock() for the whole transfer. On
large machines that lock is contended by tasks reading /proc/PID/cmdline,
/proc/PID/environ, or /proc/PID/mem, even though the memory they read is
almost always resident.

Use the per-VMA lock to access a remote process's memory, provided that
access is entirely within one VMA. Fall back to the mmap_lock if the access
crosses VMA boundaries, or when get_user_page_vma() cannot finish the
access under the per-VMA lock.

Walking the page tables under only the per-VMA lock is safe when page table
pages are freed via RCU.

pte_offset_map() takes the RCU read lock, so a PTE page freed by a
concurrent THP collapse stays valid for the walk; mid-collapse it returns
NULL and the page is faulted in.

Higher level page tables cannot be freed under us, since munmap() and
collapse take the VMA write lock that our read lock excludes.

get_user_page_vma() returns -EFAULT for memory with no struct page: the raw
PFNs of a VM_IO/VM_PFNMAP VMA, ioremapped device memory reached through
ptrace and /proc/PID/mem.

Handle that in the IS_ERR(page) branch by reaching it through
vma->vm_ops->access(), under the mmap lock, via a new
access_remote_vma_ops() helper -- as get_user_pages_remote() and the old
->access() fallback did before.

A COWed page in such a VMA has a struct page and is now returned normally
by get_user_page_vma(). Previously it was routed to ->access() and could
not be read, since generic_access_phys() ioremaps the PFN and ioremap of
RAM is rejected.

Assisted-by: Claude:claude-opus-4.8
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 mm/gup.c      |   2 +-
 mm/internal.h |   1 +
 mm/memory.c   | 169 ++++++++++++++++++++++++++++++++++++--------------
 3 files changed, 126 insertions(+), 46 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 69b834a71708..8ffcc1bbb545 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1311,7 +1311,7 @@ static bool writable_file_mapping_allowed(struct vm_area_struct *vma,
 	return !vma_needs_dirty_tracking(vma);
 }
 
-static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
+int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
 {
 	vm_flags_t vm_flags = vma->vm_flags;
 	int write = (gup_flags & FOLL_WRITE);
diff --git a/mm/internal.h b/mm/internal.h
index 0899a37907c1..3c750f2f0332 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1595,6 +1595,7 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
  */
 int __must_check try_grab_folio(struct folio *folio, int refs,
 				unsigned int flags);
+int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags);
 struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
 			       unsigned int gup_flags);
 
diff --git a/mm/memory.c b/mm/memory.c
index 3b86eeaf084f..b166b18844e9 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7014,83 +7014,162 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 EXPORT_SYMBOL_GPL(generic_access_phys);
 #endif
 
+/*
+ * VM_IO / VM_PFNMAP memory, such as an ioremapped device mapping, maps
+ * PFNs that have no struct page, so get_user_page_vma() cannot fetch it
+ * even though the page tables are populated. It can still be reached
+ * through vma->vm_ops->access().
+ *
+ * Returns the number of bytes transferred, or <= 0 if @vma cannot be
+ * accessed this way.
+ */
+static int access_remote_vma_ops(struct vm_area_struct *vma, unsigned long addr,
+				 void *buf, int len, int write)
+{
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+	if (vma->vm_ops && vma->vm_ops->access)
+		return vma->vm_ops->access(vma, addr, buf, len, write);
+#endif
+	return 0;
+}
+
 /*
  * Access another process' address space as given in mm.
+ *
+ * Use the per-VMA lock when the access fits in a single VMA, and fall back
+ * to the mmap lock for multi-VMA accesses, stack expansion, or when the
+ * VMA lock cannot be taken.
+ *
+ * Both paths fetch each page with get_user_page_vma(), which faults it in
+ * under whichever lock is held. The per-VMA path hands an access it cannot
+ * complete (a fault that dropped the lock, or a userfaultfd VMA) back to
+ * the mmap lock path.
  */
 static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
 			      void *buf, int len, unsigned int gup_flags)
 {
 	void *old_buf = buf;
 	int write = gup_flags & FOLL_WRITE;
+	bool have_mmap_lock = false;
+	struct vm_area_struct *vma = NULL;
 
-	if (mmap_read_lock_killable(mm))
-		return 0;
+	/*
+	 * Set FOLL_REMOTE so check_vma_flags() applies the same protection key
+	 * rules as get_user_pages_remote() did: the current PKRU is not checked
+	 * against a VMA reached on @mm's behalf.
+	 */
+	gup_flags |= FOLL_REMOTE;
 
-	/* Untag the address before looking up the VMA */
-	addr = untagged_addr_remote(mm, addr);
+	addr = untagged_addr_remote_unlocked(mm, addr);
 
-	/* Avoid triggering the temporary warning in __get_user_pages */
-	if (!vma_lookup(mm, addr) && !expand_stack(mm, addr))
-		return 0;
+	/*
+	 * The RCU freed page tables prevent page table memory from being
+	 * re-used and filled with unexpected contents. Ensure the access
+	 * is entirely within a single VMA.
+	 */
+#if defined(CONFIG_PER_VMA_LOCK) && defined(CONFIG_MMU_GATHER_RCU_TABLE_FREE)
+	vma = lock_vma_under_rcu(mm, addr);
+	if (vma) {
+		if (addr + len > vma->vm_end || check_vma_flags(vma, gup_flags)) {
+			vma_end_read(vma);
+			vma = NULL;
+		}
+	}
+#endif
+
+	if (!vma) {
+		if (mmap_read_lock_killable(mm))
+			return 0;
+		have_mmap_lock = true;
+	}
 
-	/* ignore errors, just check how much was successfully transferred */
 	while (len) {
+		unsigned int foll_flags = gup_flags;
+		struct page *page;
+		struct folio *folio;
 		int bytes, offset;
 		void *maddr;
-		struct folio *folio;
-		struct vm_area_struct *vma = NULL;
-		struct page *page = get_user_page_lookup_vma(mm, addr,
-							     gup_flags, &vma);
+		unsigned long idx;
 
-		if (IS_ERR(page)) {
-			/* We might need to expand the stack to access it */
+		if (!vma || addr >= vma->vm_end) {
+			/*
+			 * The per-VMA path never re-looks-up (its access fits
+			 * one VMA), so any lookup here holds the mmap lock.
+			 */
+			VM_BUG_ON(!have_mmap_lock);
 			vma = vma_lookup(mm, addr);
 			if (!vma) {
+				/* expand_stack() drops the mmap lock if it fails */
 				vma = expand_stack(mm, addr);
+				if (!vma) {
+					have_mmap_lock = false;
+					break;
+				}
+			}
+		}
 
-				/* mmap_lock was dropped on failure */
-				if (!vma)
-					return buf - old_buf;
-
-				/* Try again if stack expansion worked */
+		/*
+		 * FOLL_UNLOCKABLE lets the per-VMA fault retry, dropping the
+		 * lock, so the access can fall back to the mmap lock.
+		 */
+		if (!have_mmap_lock)
+			foll_flags |= FOLL_VMA_LOCK | FOLL_UNLOCKABLE;
+		page = get_user_page_vma(vma, addr, foll_flags);
+		if (IS_ERR(page)) {
+			/*
+			 * get_user_page_vma() returns -EAGAIN, with the per-VMA
+			 * lock released, for anything it could not finish under
+			 * it; retake the mmap lock and retry. A different error
+			 * therefore only arrives under the mmap lock, where
+			 * ->access() can run.
+			 */
+			if (PTR_ERR(page) == -EAGAIN) {
+				vma = NULL;
+				if (mmap_read_lock_killable(mm))
+					break;
+				have_mmap_lock = true;
 				continue;
 			}
-
+			if (WARN_ON_ONCE(!have_mmap_lock))
+				break;
 			/*
-			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
-			 * we can access using slightly different code.
+			 * Memory with no struct page: VM_IO / VM_PFNMAP reached
+			 * through vma->vm_ops->access(); anything else stops.
 			 */
-			bytes = 0;
-#ifdef CONFIG_HAVE_IOREMAP_PROT
-			if (vma->vm_ops && vma->vm_ops->access)
-				bytes = vma->vm_ops->access(vma, addr, buf,
-							    len, write);
-#endif
+			bytes = access_remote_vma_ops(vma, addr, buf, len, write);
 			if (bytes <= 0)
 				break;
+			goto advance;
+		}
+
+		folio = page_folio(page);
+		bytes = len;
+		offset = addr & (PAGE_SIZE - 1);
+		if (bytes > PAGE_SIZE - offset)
+			bytes = PAGE_SIZE - offset;
+
+		idx = folio_page_idx(folio, page);
+		maddr = kmap_local_folio(folio, idx * PAGE_SIZE);
+		if (write) {
+			copy_to_user_page(vma, page, addr,
+					  maddr + offset, buf, bytes);
+			folio_mark_dirty_lock(folio);
 		} else {
-			folio = page_folio(page);
-			bytes = len;
-			offset = addr & (PAGE_SIZE-1);
-			if (bytes > PAGE_SIZE-offset)
-				bytes = PAGE_SIZE-offset;
-
-			maddr = kmap_local_folio(folio, folio_page_idx(folio, page) * PAGE_SIZE);
-			if (write) {
-				copy_to_user_page(vma, page, addr,
-						  maddr + offset, buf, bytes);
-				folio_mark_dirty_lock(folio);
-			} else {
-				copy_from_user_page(vma, page, addr,
-						    buf, maddr + offset, bytes);
-			}
-			folio_release_kmap(folio, maddr);
+			copy_from_user_page(vma, page, addr,
+					    buf, maddr + offset, bytes);
 		}
+		folio_release_kmap(folio, maddr);
+
+advance:
 		len -= bytes;
 		buf += bytes;
 		addr += bytes;
 	}
-	mmap_read_unlock(mm);
+
+	if (have_mmap_lock)
+		mmap_read_unlock(mm);
+	else if (vma)
+		vma_end_read(vma);
 
 	return buf - old_buf;
 }
-- 
2.53.0-Meta


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

* [PATCH RFC v3 6/6] selftests/mm: cover /proc/pid/mem access to VM_PFNMAP memory
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
                   ` (4 preceding siblings ...)
  2026-07-17 17:00 ` [PATCH RFC v3 5/6] mm: use per-VMA lock in __access_remote_vm() for single-VMA accesses Rik van Riel
@ 2026-07-17 17:00 ` Rik van Riel
  2026-07-21 18:12 ` [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock David Hildenbrand (Arm)
  6 siblings, 0 replies; 19+ messages in thread
From: Rik van Riel @ 2026-07-17 17:00 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: kernel-team, Rik van Riel, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Shuah Khan,
	linux-kselftest

Reading a VM_PFNMAP mapping through /proc/pid/mem exercises
__access_remote_vm() two ways: a COWed page has a struct page and is
returned by get_user_page_vma(), while a raw PFN has none and is reached
through vma->vm_ops->access().

Add two tests to pfnmap.c, both reading VM_PFNMAP memory through
/proc/self/mem.

procmem_cow_read maps the file MAP_PRIVATE and writable, writes to COW a
page, then reads it back. Without the struct-page path in
get_user_page_vma() this read is short: the access falls back to
generic_access_phys(), which ioremaps the PFN, and ioremap of a COWed RAM
page is rejected.

procmem_pfn_read reads a raw PFN back through ->access(). ioremap rejects
RAM, so it runs only for genuine device memory and is skipped for the
default /dev/mem System RAM target.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 tools/testing/selftests/mm/pfnmap.c | 66 +++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
index 4f550822385a..6ff5d1029517 100644
--- a/tools/testing/selftests/mm/pfnmap.c
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -31,6 +31,7 @@ static sigjmp_buf sigjmp_buf_env;
 static char *file = "/dev/mem";
 static off_t file_offset;
 static int fd;
+static int target_is_ram;
 
 static void signal_handler(int sig)
 {
@@ -113,6 +114,7 @@ static void pfnmap_init(void)
 		if (err)
 			ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
 				       strerror(-err));
+		target_is_ram = 1;
 	} else {
 		file_offset = 0;
 	}
@@ -271,6 +273,70 @@ TEST_F(pfnmap, fork)
 	ASSERT_EQ(ret, 0);
 }
 
+TEST_F(pfnmap, procmem_cow_read)
+{
+	char *priv, *buf;
+	ssize_t rc;
+	int mem_fd;
+
+	/*
+	 * A COWed page in a VM_PFNMAP mapping has a struct page, so reading it
+	 * through /proc/self/mem -- __access_remote_vm() -> get_user_page_vma()
+	 * -- returns it directly, instead of routing to vma->vm_ops->access(),
+	 * which ioremaps the PFN and cannot reach a COWed RAM page.
+	 *
+	 * Map the file MAP_PRIVATE and writable, write to COW a page into anon
+	 * memory, then read the page back through /proc/self/mem.
+	 */
+	self->size2 = self->pagesize;
+	self->addr2 = mmap(NULL, self->size2, PROT_READ | PROT_WRITE,
+			   MAP_PRIVATE, fd, file_offset);
+	if (self->addr2 == MAP_FAILED)
+		SKIP(return, "Cannot create a writable private pfnmap mapping");
+	priv = self->addr2;
+
+	/* COW the page and stamp known bytes into the anon copy. */
+	priv[0] = 0x42;
+	priv[self->pagesize - 1] = 0x24;
+
+	buf = malloc(self->pagesize);
+	ASSERT_NE(buf, NULL);
+
+	mem_fd = open("/proc/self/mem", O_RDONLY);
+	ASSERT_GE(mem_fd, 0);
+	rc = pread(mem_fd, buf, self->pagesize, (off_t)(uintptr_t)priv);
+	close(mem_fd);
+
+	ASSERT_EQ(rc, (ssize_t)self->pagesize);
+	EXPECT_EQ(buf[0], 0x42);
+	EXPECT_EQ(buf[self->pagesize - 1], 0x24);
+
+	free(buf);
+}
+
+TEST_F(pfnmap, procmem_pfn_read)
+{
+	char buf[64];
+	ssize_t rc;
+	int mem_fd;
+
+	/*
+	 * A raw PFN of a VM_IO/VM_PFNMAP mapping has no struct page, so
+	 * __access_remote_vm() reaches it through vma->vm_ops->access()
+	 * (generic_access_phys()). That ioremaps the PFN, which is rejected for
+	 * RAM, so this only applies to genuine device memory.
+	 */
+	if (target_is_ram)
+		SKIP(return, "Target is System RAM; ->access() cannot ioremap RAM");
+
+	mem_fd = open("/proc/self/mem", O_RDONLY);
+	ASSERT_GE(mem_fd, 0);
+	rc = pread(mem_fd, buf, sizeof(buf), (off_t)(uintptr_t)self->addr1);
+	close(mem_fd);
+
+	ASSERT_EQ(rc, (ssize_t)sizeof(buf));
+}
+
 int main(int argc, char **argv)
 {
 	for (int i = 1; i < argc; i++) {
-- 
2.53.0-Meta


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

* Re: [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked()
  2026-07-17 17:00 ` [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked() Rik van Riel
@ 2026-07-20 11:12   ` Usama Arif
  0 siblings, 0 replies; 19+ messages in thread
From: Usama Arif @ 2026-07-20 11:12 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Usama Arif, linux-kernel, Andrew Morton, kernel-team,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Usama Arif

On Fri, 17 Jul 2026 13:00:31 -0400 Rik van Riel <riel@surriel.com> wrote:

> __access_remote_vm() reads another process's memory under the mmap lock. On
> large machines that lock is contended by tasks polling /proc/PID/cmdline,
> /proc/PID/environ, or calling process_vm_readv(), even though the memory
> they read is almost always resident and could be reached under the per-VMA
> lock instead.
> 
> Looking up the VMA first requires untagging the address.
> untagged_addr_remote() asserts the mmap lock only because it reads
> mm->context.untag_mask, which can race with the write in mm_enable_lam().
> 
> That mask is set once, when LAM is enabled, and never changes afterwards,
> so the read itself does not need the lock. It is already read without it,
> from the context switch path and /proc/PID/status.
> 
> Add untagged_addr_remote_unlocked() for callers that have not taken the
> mmap lock, and annotate access to mm->context.untag_mask with READ_ONCE()
> and WRITE_ONCE() so the existing lockless reads are explicit and
> KCSAN-clean. untagged_addr_remote() keeps its assertion and shares the same
> code.
> 
> Assisted-by: Claude:claude-opus-4.8
> Acked-by: Usama Arif <usamaarif642@gmail.com>

Hi Rik,

Would you mind changing this to

Acked-by: Usama Arif <usama.arif@linux.dev>

in the next revision

Thanks!
 

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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-17 17:00 ` [PATCH RFC v3 2/6] riscv/mm: " Rik van Riel
@ 2026-07-20 11:57   ` Usama Arif
  2026-07-20 15:08     ` Rik van Riel
  0 siblings, 1 reply; 19+ messages in thread
From: Usama Arif @ 2026-07-20 11:57 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Usama Arif, linux-kernel, Andrew Morton, kernel-team,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti, linux-riscv

On Fri, 17 Jul 2026 13:00:32 -0400 Rik van Riel <riel@surriel.com> wrote:

> __access_remote_vm() untags the remote address before looking up the VMA,
> now without holding the mmap lock. riscv defines untagged_addr_remote() but

This describes behavior introduced in a later patch, not by this patch.
At this point, __access_remote_vm() still acquires mmap_lock before
untagging the address.

> not untagged_addr_remote_unlocked(), so it falls back to the generic
> version, which untags with untagged_addr().
> 
> That reads current->mm, not the target mm, so a remote access to a process
> using pointer masking would untag with the wrong mask.
> 
> mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL and is stable
> afterwards, so it can be read without the mmap lock, as it already is from
> untagged_addr() and mm_untag_mask().
>

I think it might not be stable? set_tagged_addr_ctrl() can change it
repeatedly until a CLONE_VM operation sets MM_CONTEXT_LOCK_PMLEN.

> Add untagged_addr_remote_unlocked(), which untags against the target mm,
> and annotate context.pmlen accesses with READ_ONCE() and WRITE_ONCE() so
> the lockless reads are explicit and KCSAN-clean. untagged_addr_remote()
> keeps its mmap_assert_locked() and shares the code.
> 
> Assisted-by: Claude:claude-opus-4.8
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  arch/riscv/include/asm/mmu_context.h |  4 ++--
>  arch/riscv/include/asm/uaccess.h     | 10 +++++++---
>  arch/riscv/kernel/process.c          | 12 +++++++-----
>  3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
> index dbf27a78df6c..3ce16796e5a2 100644
> --- a/arch/riscv/include/asm/mmu_context.h
> +++ b/arch/riscv/include/asm/mmu_context.h
> @@ -21,7 +21,7 @@ static inline void activate_mm(struct mm_struct *prev,
>  			       struct mm_struct *next)
>  {
>  #ifdef CONFIG_RISCV_ISA_SUPM
> -	next->context.pmlen = 0;
> +	WRITE_ONCE(next->context.pmlen, 0);
>  #endif
>  	switch_mm(prev, next, NULL);
>  }
> @@ -44,7 +44,7 @@ DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
>  #define mm_untag_mask mm_untag_mask
>  static inline unsigned long mm_untag_mask(struct mm_struct *mm)
>  {
> -	return -1UL >> mm->context.pmlen;
> +	return -1UL >> READ_ONCE(mm->context.pmlen);
>  }
>  #endif
>  
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index 5d4ec15584cf..53806e0f7dcf 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -16,7 +16,7 @@
>  static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigned long addr)
>  {
>  	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM)) {
> -		u8 pmlen = mm->context.pmlen;
> +		u8 pmlen = READ_ONCE(mm->context.pmlen);
>  
>  		/* Virtual addresses are sign-extended; physical addresses are zero-extended. */
>  		if (IS_ENABLED(CONFIG_MMU))
> @@ -33,12 +33,16 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
>  	(__force __typeof__(addr))__untagged_addr_remote(current->mm, __addr);	\
>  })
>  
> -#define untagged_addr_remote(mm, addr) ({					\
> +#define untagged_addr_remote_unlocked(mm, addr) ({				\
>  	unsigned long __addr = (__force unsigned long)(addr);			\
> -	mmap_assert_locked(mm);							\
>  	(__force __typeof__(addr))__untagged_addr_remote(mm, __addr);		\
>  })
>  
> +#define untagged_addr_remote(mm, addr) ({					\
> +	mmap_assert_locked(mm);							\
> +	untagged_addr_remote_unlocked(mm, addr);				\
> +})
> +
>  #define access_ok(addr, size) likely(__access_ok(untagged_addr(addr), size))
>  #else
>  #define untagged_addr(addr) (addr)
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index b2df7f72241a..6ae7552fed09 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -357,13 +357,15 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg)
>  	if (mmap_write_lock_killable(mm))
>  		return -EINTR;
>  
> -	if (test_bit(MM_CONTEXT_LOCK_PMLEN, &mm->context.flags) && mm->context.pmlen != pmlen) {
> -		mmap_write_unlock(mm);
> -		return -EBUSY;
> +	if (test_bit(MM_CONTEXT_LOCK_PMLEN, &mm->context.flags)) {
> +		if (READ_ONCE(mm->context.pmlen) != pmlen) {
> +			mmap_write_unlock(mm);
> +			return -EBUSY;
> +		}
>  	}
>  
>  	envcfg_update_bits(task, ENVCFG_PMM, pmm);
> -	mm->context.pmlen = pmlen;
> +	WRITE_ONCE(mm->context.pmlen, pmlen);
>  
>  	mmap_write_unlock(mm);
>  
> @@ -394,7 +396,7 @@ long get_tagged_addr_ctrl(struct task_struct *task)
>  		break;
>  	}
>  
> -	if (task->mm->context.pmlen)
> +	if (READ_ONCE(task->mm->context.pmlen))
>  		ret |= PR_TAGGED_ADDR_ENABLE;
>  
>  	return ret;
> -- 
> 2.53.0-Meta
> 
> 

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

* Re: [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma()
  2026-07-17 17:00 ` [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma() Rik van Riel
@ 2026-07-20 12:00   ` Usama Arif
  0 siblings, 0 replies; 19+ messages in thread
From: Usama Arif @ 2026-07-20 12:00 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Usama Arif, linux-kernel, Andrew Morton, kernel-team,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, Catalin Marinas, Will Deacon, Masami Hiramatsu,
	Oleg Nesterov, Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Harry Yoo,
	Jann Horn, Lance Yang, linux-arm-kernel, linux-trace-kernel

On Fri, 17 Jul 2026 13:00:33 -0400 Rik van Riel <riel@surriel.com> wrote:

> get_user_page_vma_remote() faults in the page at @addr in a remote mm and
> also looks up the VMA that covers it, handing both back to the caller.
> 
> This cleans up the name space for adding a get_user_page_vma() variant

nit: s/name space/namespace/

> where the caller already has the vma.
> 
> Assisted-by: Claude:claude-opus-4.8
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  arch/arm64/kernel/mte.c   | 2 +-
>  arch/x86/kernel/uprobes.c | 2 +-
>  include/linux/mm.h        | 2 +-
>  mm/memory.c               | 4 ++--
>  mm/rmap.c                 | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 


I think good to add no functional change intended in the patch.

Acked-by: Usama Arif <usama.arif@linux.dev>


> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 1a9aad6ef22a..7a6ecc3d9294 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -459,7 +459,7 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,
>  		struct vm_area_struct *vma;
>  		unsigned long tags, offset;
>  		void *maddr;
> -		struct page *page = get_user_page_vma_remote(mm, addr,
> +		struct page *page = get_user_page_lookup_vma(mm, addr,
>  							     gup_flags, &vma);
>  		struct folio *folio;
>  
> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index 3af979fb41d3..329efac0cfb3 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
> @@ -1036,7 +1036,7 @@ static int copy_from_vaddr(struct mm_struct *mm, unsigned long vaddr, void *dst,
>  	struct vm_area_struct *vma;
>  	struct page *page;
>  
> -	page = get_user_page_vma_remote(mm, vaddr, gup_flags, &vma);
> +	page = get_user_page_lookup_vma(mm, vaddr, gup_flags, &vma);
>  	if (IS_ERR(page))
>  		return PTR_ERR(page);
>  	uprobe_copy_from_page(page, vaddr, dst, len);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 485df9c2dbdd..24ead14b4790 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3238,7 +3238,7 @@ long pin_user_pages_remote(struct mm_struct *mm,
>  /*
>   * Retrieves a single page alongside its VMA. Does not support FOLL_NOWAIT.
>   */
> -static inline struct page *get_user_page_vma_remote(struct mm_struct *mm,
> +static inline struct page *get_user_page_lookup_vma(struct mm_struct *mm,
>  						    unsigned long addr,
>  						    int gup_flags,
>  						    struct vm_area_struct **vmap)
> diff --git a/mm/memory.c b/mm/memory.c
> index ff338c2abe92..3b86eeaf084f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -7039,7 +7039,7 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
>  		void *maddr;
>  		struct folio *folio;
>  		struct vm_area_struct *vma = NULL;
> -		struct page *page = get_user_page_vma_remote(mm, addr,
> +		struct page *page = get_user_page_lookup_vma(mm, addr,
>  							     gup_flags, &vma);
>  
>  		if (IS_ERR(page)) {
> @@ -7167,7 +7167,7 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
>  		struct page *page;
>  		struct vm_area_struct *vma = NULL;
>  
> -		page = get_user_page_vma_remote(mm, addr, gup_flags, &vma);
> +		page = get_user_page_lookup_vma(mm, addr, gup_flags, &vma);
>  		if (IS_ERR(page)) {
>  			/*
>  			 * Treat as a total failure for now until we decide how
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1c77d5dc06e9..b36f2e219b8f 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2838,7 +2838,7 @@ struct page *make_device_exclusive(struct mm_struct *mm, unsigned long addr,
>  	 * (non-device-exclusive) PTE and issue a MMU_NOTIFY_EXCLUSIVE.
>  	 */
>  retry:
> -	page = get_user_page_vma_remote(mm, addr,
> +	page = get_user_page_lookup_vma(mm, addr,
>  					FOLL_GET | FOLL_WRITE | FOLL_SPLIT_PMD,
>  					&vma);
>  	if (IS_ERR(page))
> -- 
> 2.53.0-Meta
> 
> 

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

* Re: [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock
  2026-07-17 17:00 ` [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock Rik van Riel
@ 2026-07-20 12:35   ` Usama Arif
  2026-07-20 15:24     ` Rik van Riel
  0 siblings, 1 reply; 19+ messages in thread
From: Usama Arif @ 2026-07-20 12:35 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Usama Arif, linux-kernel, Andrew Morton, kernel-team,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, Jason Gunthorpe, John Hubbard, Peter Xu

On Fri, 17 Jul 2026 13:00:34 -0400 Rik van Riel <riel@surriel.com> wrote:

> __access_remote_vm() needs a single page from a VMA it has already
> looked up and locked, faulting it in when necessary, under either the
> mmap lock or the per-VMA lock. get_user_pages_remote() does not fit: it
> hard codes the mmap lock and re-looks-up the VMA, neither of which is
> wanted here.
> 
> Add get_user_page_vma(), a simplified __get_user_pages() that walks the
> page tables with follow_page_mask(), faults a missing page in with
> faultin_page(), and on success returns it with a reference and the
> caller's lock still held. Like __get_user_pages() it runs
> check_vma_flags(), so callers need not pre-check the VMA.
> 
> A VM_IO/VM_PFNMAP VMA is the exception to that check: it can still hold
> COWed pages that have a struct page, so follow_page_mask() is allowed to
> look for one. Memory with no struct page -- a raw PFN, or a present PFN
> reported as -EEXIST -- is returned as -EFAULT, so the caller can reach it
> through vma->vm_ops->access().
> 
> The caller sets FOLL_VMA_LOCK when it holds the per-VMA lock rather than
> the mmap lock, which reaches the fault code as FAULT_FLAG_VMA_LOCK.
> 
> Anything that cannot complete under the per-VMA lock -- a dropped fault,
> a userfaultfd VMA (uffd assumes current is the faulting task), a hard
> error, or ->access() memory -- releases the lock and returns -EAGAIN,
> so the caller retries under the mmap lock.
> 
> faultin_page() reports these retries as -EAGAIN for both lock types;
> only the mmap caller records the dropped lock in *locked. A
> VM_FAULT_ERROR that decodes to no errno warns and returns -EFAULT
> rather than BUG(), since the mmap-lock retry produces the definitive
> result.
> 
> Assisted-by: Claude:claude-opus-4.8
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  mm/gup.c      | 154 +++++++++++++++++++++++++++++++++++++++++++-------
>  mm/internal.h |   6 +-
>  2 files changed, 139 insertions(+), 21 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 0692119b7904..69b834a71708 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1082,7 +1082,13 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address,
>  /*
>   * mmap_lock must be held on entry.  If @flags has FOLL_UNLOCKABLE but not

Does the above line also need to be changed? mmap_lock is not held on entry in
faultin_page() if passing FOLL_VMA_LOCK, right?

>   * FOLL_NOWAIT, the mmap_lock may be released.  If it is, *@locked will be set
> - * to 0 and -EBUSY returned.
> + * to 0 and -EAGAIN returned.
> + *
> + * The return value does not depend on the lock type: a fault that made
> + * progress but needs a retry (VM_FAULT_RETRY / VM_FAULT_COMPLETED) is reported
> + * as -EAGAIN for both the mmap lock and the per-VMA lock (FOLL_VMA_LOCK). Only
> + * the *@locked side effect is lock-type specific, as the per-VMA lock path has
> + * no unlockable mmap_lock to drop.
>   */
>  static int faultin_page(struct vm_area_struct *vma,
>  		unsigned long address, unsigned int flags, bool unshare,
> @@ -1097,6 +1103,8 @@ static int faultin_page(struct vm_area_struct *vma,
>  		fault_flags |= FAULT_FLAG_WRITE;
>  	if (flags & FOLL_REMOTE)
>  		fault_flags |= FAULT_FLAG_REMOTE;
> +	if (flags & FOLL_VMA_LOCK)
> +		fault_flags |= FAULT_FLAG_VMA_LOCK;
>  	if (flags & FOLL_UNLOCKABLE) {
>  		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
>  		/*
> @@ -1125,41 +1133,147 @@ static int faultin_page(struct vm_area_struct *vma,
>  
>  	ret = handle_mm_fault(vma, address, fault_flags, NULL);
>  
> +	/*
> +	 * A fully completed fault (VM_FAULT_COMPLETED) or one that needs a retry
> +	 * (VM_FAULT_RETRY) has released the lock it was holding. Report both as
> +	 * -EAGAIN so the caller retries: the mmap lock caller retakes it here,
> +	 * the per-VMA lock caller (FOLL_VMA_LOCK) falls back to the mmap lock.
> +	 *
> +	 * Dropping the mmap lock is recorded in *@locked. There is no such lock
> +	 * to drop under the per-VMA lock, where @locked is not used, so leave it
> +	 * alone in that case.
> +	 */
>  	if (ret & VM_FAULT_COMPLETED) {
> -		/*
> -		 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
> -		 * mmap lock in the page fault handler. Sanity check this.
> -		 */
> -		WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
> -		*locked = 0;
> -
> -		/*
> -		 * We should do the same as VM_FAULT_RETRY, but let's not
> -		 * return -EBUSY since that's not reflecting the reality of
> -		 * what has happened - we've just fully completed a page
> -		 * fault, with the mmap lock released.  Use -EAGAIN to show
> -		 * that we want to take the mmap lock _again_.
> -		 */
> +		if (!(flags & FOLL_VMA_LOCK)) {
> +			/*
> +			 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
> +			 * mmap lock in the page fault handler. Sanity check this.
> +			 */
> +			WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
> +			*locked = 0;
> +		}
>  		return -EAGAIN;
>  	}
>  
>  	if (ret & VM_FAULT_ERROR) {
>  		int err = vm_fault_to_errno(ret, flags);
>  
> -		if (err)
> -			return err;
> -		BUG();
> +		/*
> +		 * VM_FAULT_ERROR always decodes to an errno; a zero here would
> +		 * mean handle_mm_fault() returned an unexpected combination.
> +		 * Report -EFAULT rather than crash: under the per-VMA lock the
> +		 * mmap lock retry produces the definitive result.
> +		 */
> +		VM_WARN_ON_ONCE(!err);
> +		return err ? err : -EFAULT;
>  	}
>  
>  	if (ret & VM_FAULT_RETRY) {
> -		if (!(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
> +		if (!(flags & FOLL_VMA_LOCK) &&
> +		    !(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
>  			*locked = 0;
> -		return -EBUSY;
> +		return -EAGAIN;
>  	}
>  
>  	return 0;
>  }
>  
> +/*
> + * get_user_page_vma - get one page from @vma, whose lock the caller already
> + * holds: the mmap lock, or (with FOLL_VMA_LOCK) the per-VMA lock. Walks the
> + * page tables, faulting the page in if needed, and on success returns it with
> + * a reference and the lock still held.
> + *
> + * Runs check_vma_flags() like __get_user_pages(), so callers need not pre-check
> + * the VMA; most rejections are returned as their error. A VM_IO/VM_PFNMAP VMA
> + * is the exception: a COWed page with a struct page is returned, while a raw
> + * PFN has none and yields -EFAULT, to be reached via vma->vm_ops->access().
> + *
> + * Under FOLL_VMA_LOCK, anything that cannot be finished under the per-VMA lock
> + * (a dropped fault, userfaultfd, a hard error, or ->access() memory) releases
> + * the lock and returns -EAGAIN, so the caller retries under the mmap lock.
> + */
> +struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
> +			       unsigned int gup_flags)
> +{
> +	bool vma_locked = gup_flags & FOLL_VMA_LOCK;
> +	unsigned long page_mask;
> +	struct page *page;
> +	int locked = 1;
> +	bool pfnmap;
> +	int ret;
> +
> +	/*
> +	 * Validate the VMA up front, like __get_user_pages(). A VM_IO/VM_PFNMAP
> +	 * VMA is not rejected outright: it can hold COWed pages that have a
> +	 * struct page, so let follow_page_mask() look for one, and treat only
> +	 * its struct-page-less PFNs as unreachable. Any other rejection
> +	 * (secretmem, bad permissions, ...) is final.
> +	 */
> +	ret = check_vma_flags(vma, gup_flags);
> +	if (ret && !(vma->vm_flags & (VM_IO | VM_PFNMAP)))

Do you need to somehow restructure check_vma_flags()?

The first thing that check_vma_flags() does is:

	if (vm_flags & (VM_IO | VM_PFNMAP))
		return -EFAULT;

check_vma_flags() returns before evaluating VM_READ,
FOLL_FORCE, FOLL_ANON or the architecture permission check.

The code then ignores that early error and may return a COWed
page through follow_page_mask(). A non-FOLL_FORCE caller
can therefore read a COWed page from a PROT_NONE PFNMAP VMA.

> +		goto fail;
> +	pfnmap = ret;
> +
> +	for (;;) {
> +		if (fatal_signal_pending(current)) {
> +			ret = -EINTR;
> +			goto fail;
> +		}
> +		cond_resched();
> +
> +		page = follow_page_mask(vma, addr,
> +					gup_flags | FOLL_TOUCH | FOLL_GET,
> +					&page_mask);
> +		if (!IS_ERR_OR_NULL(page))
> +			return page;

__get_user_pages() does

	flush_anon_page(vma, subpage, start + j * PAGE_SIZE);
	flush_dcache_page(subpage);

before returning the page. Do you need to that here as well above?

> +
> +		/*
> +		 * No struct page: a raw PFN of a VM_IO/VM_PFNMAP VMA, whether
> +		 * seen by the up-front check (@pfnmap) or reported as -EEXIST
> +		 * for a present PFN. Return -EFAULT so the caller reaches it
> +		 * through vma->vm_ops->access().
> +		 */
> +		if (pfnmap || PTR_ERR(page) == -EEXIST) {
> +			ret = -EFAULT;
> +			goto fail;
> +		}
> +		/* A hard error from the walk itself. */
> +		if (page && PTR_ERR(page) != -EMLINK) {
> +			ret = PTR_ERR(page);
> +			goto fail;
> +		}
> +
> +		/*
> +		 * The page is not present, or needs unsharing. A remote fault
> +		 * under the per-VMA lock cannot deliver userfaultfd (which
> +		 * assumes current is the faulting task), so fall back for those.
> +		 */
> +		if (vma_locked && userfaultfd_armed(vma)) {
> +			ret = -EAGAIN;
> +			goto fail;
> +		}
> +		ret = faultin_page(vma, addr, gup_flags | FOLL_REMOTE | FOLL_GET,
> +				   PTR_ERR(page) == -EMLINK, &locked);
> +		if (ret == -EAGAIN)
> +			return ERR_PTR(-EAGAIN);	/* fault released the per-VMA lock */
> +		if (ret)
> +			goto fail;
> +	}
> +
> +fail:
> +	/*
> +	 * Under the per-VMA lock the caller cannot reach ->access() or act on a
> +	 * hard error (both need the mmap lock), so release the lock and have it
> +	 * retry there; the mmap-lock pass produces the definitive error.
> +	 */
> +	if (vma_locked) {
> +		vma_end_read(vma);
> +		return ERR_PTR(-EAGAIN);
> +	}
> +	return ERR_PTR(ret);
> +}
> +
>  /*
>   * Writing to file-backed mappings which require folio dirty tracking using GUP
>   * is a fundamentally broken operation, as kernel write access to GUP mappings
> diff --git a/mm/internal.h b/mm/internal.h
> index 181e79f1d6a2..0899a37907c1 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1595,6 +1595,8 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
>   */
>  int __must_check try_grab_folio(struct folio *folio, int refs,
>  				unsigned int flags);
> +struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
> +			       unsigned int gup_flags);
>  
>  /*
>   * mm/huge_memory.c
> @@ -1641,11 +1643,13 @@ enum {
>  	FOLL_UNLOCKABLE = 1 << 21,
>  	/* VMA lookup+checks compatible with MADV_POPULATE_(READ|WRITE) */
>  	FOLL_MADV_POPULATE = 1 << 22,
> +	/* caller holds the per-VMA lock, not the mmap lock */
> +	FOLL_VMA_LOCK = 1 << 23,
>  };
>  
>  #define INTERNAL_GUP_FLAGS (FOLL_TOUCH | FOLL_TRIED | FOLL_REMOTE | FOLL_PIN | \
>  			    FOLL_FAST_ONLY | FOLL_UNLOCKABLE | \
> -			    FOLL_MADV_POPULATE)
> +			    FOLL_MADV_POPULATE | FOLL_VMA_LOCK)
>  
>  /*
>   * Indicates for which pages that are write-protected in the page table,
> -- 
> 2.53.0-Meta
> 
> 

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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 11:57   ` Usama Arif
@ 2026-07-20 15:08     ` Rik van Riel
  2026-07-20 16:46       ` Usama Arif
  0 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-20 15:08 UTC (permalink / raw)
  To: Usama Arif
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv

On Mon, 2026-07-20 at 04:57 -0700, Usama Arif wrote:
> On Fri, 17 Jul 2026 13:00:32 -0400 Rik van Riel <riel@surriel.com>
> wrote:
> 
> 
> > 
> > mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL and
> > is stable
> > afterwards, so it can be read without the mmap lock, as it already
> > is from
> > untagged_addr() and mm_untag_mask().
> > 
> 
> I think it might not be stable? set_tagged_addr_ctrl() can change it
> repeatedly until a CLONE_VM operation sets MM_CONTEXT_LOCK_PMLEN.

You're right, ARM tagged addresses seem to work a
little differently from x86 LAM.

It looks like on ARM with MTE, the top 8 bits of
the virtual address are available as tags, meaning
that address space cannot be used for VMAs.

For purposes of accessing process memory, this
is a little more stable than LAM, because the
number of bits we need to mask out of the address
is always the same when MTE is in use.

At least, I think so. Am I overlooking anything?

I will clean up the changelog like you suggested.

-- 
All Rights Reversed.

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

* Re: [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock
  2026-07-20 12:35   ` Usama Arif
@ 2026-07-20 15:24     ` Rik van Riel
  0 siblings, 0 replies; 19+ messages in thread
From: Rik van Riel @ 2026-07-20 15:24 UTC (permalink / raw)
  To: Usama Arif
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Jason Gunthorpe,
	John Hubbard, Peter Xu

On Mon, 2026-07-20 at 05:35 -0700, Usama Arif wrote:
> On Fri, 17 Jul 2026 13:00:34 -0400 Rik van Riel <riel@surriel.com>
> wrote:
> 
> > 
> > +	/*
> > +	 * Validate the VMA up front, like __get_user_pages(). A
> > VM_IO/VM_PFNMAP
> > +	 * VMA is not rejected outright: it can hold COWed pages
> > that have a
> > +	 * struct page, so let follow_page_mask() look for one,
> > and treat only
> > +	 * its struct-page-less PFNs as unreachable. Any other
> > rejection
> > +	 * (secretmem, bad permissions, ...) is final.
> > +	 */
> > +	ret = check_vma_flags(vma, gup_flags);
> > +	if (ret && !(vma->vm_flags & (VM_IO | VM_PFNMAP)))
> 
> Do you need to somehow restructure check_vma_flags()?
> 
> The first thing that check_vma_flags() does is:
> 
> 	if (vm_flags & (VM_IO | VM_PFNMAP))
> 		return -EFAULT;
> 
> check_vma_flags() returns before evaluating VM_READ,
> FOLL_FORCE, FOLL_ANON or the architecture permission check.
> 
> The code then ignores that early error and may return a COWed
> page through follow_page_mask(). A non-FOLL_FORCE caller
> can therefore read a COWed page from a PROT_NONE PFNMAP VMA.

In this code path, if we return an error to
__access_remote_vm(), we end up falling back
to the ->access() hook, and will end up reading
the memory that's "under" that COW, instead of
the COWed page that is currently in the process
we are accessing.

In other words, I think the behavior of this
code is the way we want, even though the
implementation should be cleaned up.

How about we change check_vma_flags() so it
ignores specified flags?

check_vma_flags(vma, gup_flags, ignore_flags)

Then this one call to check_vma_flags can have
it ignore VM_IO | VM_PFNMAP, since we want to
access the COWed pages in those VMAs in this
code path, but fall back to ->access when
there are no COW pages.

> 
> > +		goto fail;
> > +	pfnmap = ret;
> > +
> > +	for (;;) {
> > +		if (fatal_signal_pending(current)) {
> > +			ret = -EINTR;
> > +			goto fail;
> > +		}
> > +		cond_resched();
> > +
> > +		page = follow_page_mask(vma, addr,
> > +					gup_flags | FOLL_TOUCH |
> > FOLL_GET,
> > +					&page_mask);
> > +		if (!IS_ERR_OR_NULL(page))
> > +			return page;
> 
> __get_user_pages() does
> 
> 	flush_anon_page(vma, subpage, start + j * PAGE_SIZE);
> 	flush_dcache_page(subpage);
> 
> before returning the page. Do you need to that here as well above?

I suppose on systems with virtually indexed caches
we do need that.

Thanks for spotting that!

Now I also wonder if I could restructure get_user_page_vma_remote()
to do the lookup before the get_user_pages_remote(), and have
it then use the new get_user_page_vma() function, to avoid
the double VMA lookup (why are we doing that, anyway?)
> 

-- 
All Rights Reversed.

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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 15:08     ` Rik van Riel
@ 2026-07-20 16:46       ` Usama Arif
  2026-07-20 17:34         ` Rik van Riel
  0 siblings, 1 reply; 19+ messages in thread
From: Usama Arif @ 2026-07-20 16:46 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv



On 20/07/2026 16:08, Rik van Riel wrote:
> On Mon, 2026-07-20 at 04:57 -0700, Usama Arif wrote:
>> On Fri, 17 Jul 2026 13:00:32 -0400 Rik van Riel <riel@surriel.com>
>> wrote:
>>
>>
>>>
>>> mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL and
>>> is stable
>>> afterwards, so it can be read without the mmap lock, as it already
>>> is from
>>> untagged_addr() and mm_untag_mask().
>>>
>>
>> I think it might not be stable? set_tagged_addr_ctrl() can change it
>> repeatedly until a CLONE_VM operation sets MM_CONTEXT_LOCK_PMLEN.
> 
> You're right, ARM tagged addresses seem to work a

ah do you mean RISCV here?

> little differently from x86 LAM.
> 
> It looks like on ARM with MTE, the top 8 bits of
> the virtual address are available as tags, meaning
> that address space cannot be used for VMAs.
> 
> For purposes of accessing process memory, this
> is a little more stable than LAM, because the
> number of bits we need to mask out of the address
> is always the same when MTE is in use.
> 
> At least, I think so. Am I overlooking anything?
> 
> I will clean up the changelog like you suggested.
> 


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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 16:46       ` Usama Arif
@ 2026-07-20 17:34         ` Rik van Riel
  2026-07-20 18:46           ` Usama Arif
  0 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-20 17:34 UTC (permalink / raw)
  To: Usama Arif
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv

On Mon, 2026-07-20 at 17:46 +0100, Usama Arif wrote:
> 
> 
> On 20/07/2026 16:08, Rik van Riel wrote:
> > On Mon, 2026-07-20 at 04:57 -0700, Usama Arif wrote:
> > > On Fri, 17 Jul 2026 13:00:32 -0400 Rik van Riel
> > > <riel@surriel.com>
> > > wrote:
> > > 
> > > 
> > > > 
> > > > mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL
> > > > and
> > > > is stable
> > > > afterwards, so it can be read without the mmap lock, as it
> > > > already
> > > > is from
> > > > untagged_addr() and mm_untag_mask().
> > > > 
> > > 
> > > I think it might not be stable? set_tagged_addr_ctrl() can change
> > > it
> > > repeatedly until a CLONE_VM operation sets MM_CONTEXT_LOCK_PMLEN.
> > 
> > You're right, ARM tagged addresses seem to work a
> 
> ah do you mean RISCV here?

Ugh, I looked at the wrong one.  RISCV is like
x86, with a dynamic (though a limited number
of options?) mask.

Having said that, won't a process have all of
its VMAs in the masked-off area, so coming in
from a remote process to a valid memory address
should already give us a valid address?

-- 
All Rights Reversed.

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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 17:34         ` Rik van Riel
@ 2026-07-20 18:46           ` Usama Arif
  2026-07-20 19:21             ` Rik van Riel
  0 siblings, 1 reply; 19+ messages in thread
From: Usama Arif @ 2026-07-20 18:46 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv



On 20/07/2026 18:34, Rik van Riel wrote:
> On Mon, 2026-07-20 at 17:46 +0100, Usama Arif wrote:
>>
>>
>> On 20/07/2026 16:08, Rik van Riel wrote:
>>> On Mon, 2026-07-20 at 04:57 -0700, Usama Arif wrote:
>>>> On Fri, 17 Jul 2026 13:00:32 -0400 Rik van Riel
>>>> <riel@surriel.com>
>>>> wrote:
>>>>
>>>>
>>>>>
>>>>> mm->context.pmlen is set only through PR_SET_TAGGED_ADDR_CTRL
>>>>> and
>>>>> is stable
>>>>> afterwards, so it can be read without the mmap lock, as it
>>>>> already
>>>>> is from
>>>>> untagged_addr() and mm_untag_mask().
>>>>>
>>>>
>>>> I think it might not be stable? set_tagged_addr_ctrl() can change
>>>> it
>>>> repeatedly until a CLONE_VM operation sets MM_CONTEXT_LOCK_PMLEN.
>>>
>>> You're right, ARM tagged addresses seem to work a
>>
>> ah do you mean RISCV here?
> 
> Ugh, I looked at the wrong one.  RISCV is like
> x86, with a dynamic (though a limited number
> of options?) mask.
> 
> Having said that, won't a process have all of
> its VMAs in the masked-off area, so coming in
> from a remote process to a valid memory address
> should already give us a valid address?
> 

So my understanding from exploring this code is, and hopefully someone
in CC from riscv can correct me, for example:

Tagged pointer:  0xabcd000012345678
PMLEN 16:        0x0000000012345678
PMLEN 7:         0xffcd000012345678

The target VMA might be at 0x12345678, but applying PMLEN 7
to that tagged pointer does not produce that address.

Previously, the order was:

Take mmap read lock.
Read pmlen.
Untag the address.
Look up the VMA.

Changing PMLEN takes the mmap write lock. The read and write
operations were therefore serialized.

The new order is:

Read pmlen without mmap lock.
Untag the address.
Attempt the per-VMA lookup.
Possibly take mmap lock later.
Continue using the already-untagged address.

A concurrent PMLEN change can occur between those operations?



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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 18:46           ` Usama Arif
@ 2026-07-20 19:21             ` Rik van Riel
  2026-07-20 19:39               ` Usama Arif
  0 siblings, 1 reply; 19+ messages in thread
From: Rik van Riel @ 2026-07-20 19:21 UTC (permalink / raw)
  To: Usama Arif
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv

On Mon, 2026-07-20 at 19:46 +0100, Usama Arif wrote:
> 
> So my understanding from exploring this code is, and hopefully
> someone
> in CC from riscv can correct me, for example:
> 
> Tagged pointer:  0xabcd000012345678
> PMLEN 16:        0x0000000012345678
> PMLEN 7:         0xffcd000012345678
> 
> The target VMA might be at 0x12345678, but applying PMLEN 7
> to that tagged pointer does not produce that address.
> 
> Previously, the order was:
> 
> Take mmap read lock.
> Read pmlen.
> Untag the address.
> Look up the VMA.
> 
> Changing PMLEN takes the mmap write lock. The read and write
> operations were therefore serialized.
> 
> The new order is:
> 
> Read pmlen without mmap lock.
> Untag the address.
> Attempt the per-VMA lookup.
> Possibly take mmap lock later.
> Continue using the already-untagged address.
> 
> A concurrent PMLEN change can occur between those operations?

I suppose it could, but what are the possible outcomes here?

- We fail to untag the address, the vma lookup
  fails, and we fail to access memory.

- The address is already untagged, maps to a
  VMA, and the access succeeds.

Are there any others?

The VMAs of the process need to be in the bottom
part of the address space, right? The part where
untagged addresses sit.

For things like /proc/<pid>/cmdline we should
automatically get an address without any of the
high bits set.

For things like ptrace peek / poke, BPF process
accesses, and others, I really do not know if
those could get tagged addresses...

What are the failures we need to protect against?

What if something comes in with a tagged address,
but the process disables tagging while that
something waits for the mmap_lock?

Does that reproduce the failure case, without
any locking changes?
> 

> 

-- 
All Rights Reversed.

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

* Re: [PATCH RFC v3 2/6] riscv/mm: add untagged_addr_remote_unlocked()
  2026-07-20 19:21             ` Rik van Riel
@ 2026-07-20 19:39               ` Usama Arif
  0 siblings, 0 replies; 19+ messages in thread
From: Usama Arif @ 2026-07-20 19:39 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, Andrew Morton, kernel-team, David Hildenbrand,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv



On 20/07/2026 20:21, Rik van Riel wrote:
> On Mon, 2026-07-20 at 19:46 +0100, Usama Arif wrote:
>>
>> So my understanding from exploring this code is, and hopefully
>> someone
>> in CC from riscv can correct me, for example:
>>
>> Tagged pointer:  0xabcd000012345678
>> PMLEN 16:        0x0000000012345678
>> PMLEN 7:         0xffcd000012345678
>>
>> The target VMA might be at 0x12345678, but applying PMLEN 7
>> to that tagged pointer does not produce that address.
>>
>> Previously, the order was:
>>
>> Take mmap read lock.
>> Read pmlen.
>> Untag the address.
>> Look up the VMA.
>>
>> Changing PMLEN takes the mmap write lock. The read and write
>> operations were therefore serialized.
>>
>> The new order is:
>>
>> Read pmlen without mmap lock.
>> Untag the address.
>> Attempt the per-VMA lookup.
>> Possibly take mmap lock later.
>> Continue using the already-untagged address.
>>
>> A concurrent PMLEN change can occur between those operations?
> 
> I suppose it could, but what are the possible outcomes here?
> 
> - We fail to untag the address, the vma lookup
>   fails, and we fail to access memory.
> 
> - The address is already untagged, maps to a
>   VMA, and the access succeeds.
> 
> Are there any others?
> 
> The VMAs of the process need to be in the bottom
> part of the address space, right? The part where
> untagged addresses sit.
> 
> For things like /proc/<pid>/cmdline we should
> automatically get an address without any of the
> high bits set.
> 
> For things like ptrace peek / poke, BPF process
> accesses, and others, I really do not know if
> those could get tagged addresses...
> 
> What are the failures we need to protect against?
> 
> What if something comes in with a tagged address,
> but the process disables tagging while that
> something waits for the mmap_lock?

So I think the above question is what needs to be
answered.
A tagged pointer can become invalid if PMLEN changes
before the old mmap-locked lookup too.
The mmap lock only defined whether the lookup observed
the old or new mode.

For VMA as you said, it should be ok. I don't know about
others. I think it would be best to get input from
RISC-V folks for this. Hopefully its ok..
If it is ok, then all that would be need to be done
is to just remove in the commit message that pmlen
is stable.

> 
> Does that reproduce the failure case, without
> any locking changes?




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

* Re: [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock
  2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
                   ` (5 preceding siblings ...)
  2026-07-17 17:00 ` [PATCH RFC v3 6/6] selftests/mm: cover /proc/pid/mem access to VM_PFNMAP memory Rik van Riel
@ 2026-07-21 18:12 ` David Hildenbrand (Arm)
  6 siblings, 0 replies; 19+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-21 18:12 UTC (permalink / raw)
  To: Rik van Riel, linux-kernel, Andrew Morton
  Cc: kernel-team, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
	Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-mm

On 7/17/26 19:00, Rik van Riel wrote:
> __access_remote_vm() holds mmap_read_lock() for the whole transfer. On
> large machines with large multi-threaded applications, the mmap_lock
> is often contended due to mixed accesses from readers and writers, like
> mmap and munmap. When a lock holder gets stuck, system monitoring software
> can get stuck behind that, resulting in a failure to log that the system
> is in trouble.
> 
> Take the per-VMA lock in __access_remote_vm() when the access falls
> entirely within a single VMA. Fall back to the mmap lock when the access
> crosses a VMA boundary, or when the page cannot be reached under the
> per-VMA lock: a dropped fault, a userfaultfd VMA, a hard error, or memory
> with no struct page that has to go through vma->vm_ops->access().
> 
> The bulk of the work is a new gup helper. __access_remote_vm() needs a
> single page from a VMA it has already looked up and locked, faulting it in
> when necessary, under either lock.

Yes, much better.

> 
> get_user_pages_remote() does not fit: it hard codes the mmap lock and
> re-derives the VMA. get_user_page_vma() walks the page tables, faults a
> missing page in, and returns it with a reference and the caller's lock
> still held.

Agreed, and that can be tackled later.

> 
> The per-VMA path also closes a pre-existing gap. A COWed page in a
> VM_IO/VM_PFNMAP VMA has a struct page, but the old code routed it to
> ->access(), where generic_access_phys() ioremaps the PFN and ioremap of RAM
> is rejected, so the read came up short.

Yep. The GUP code rejected VM_IO/VM_PFNMAP for, though, because it did not
properly handle vm_normal_page_pmd() etc so far.

> 
> get_user_page_vma() now returns that page normally. Raw PFNs with no struct
> page still reach ->access() under the mmap lock, as before.
> 
> The series is arranged as:


It will take me a bit to dig into the details; I'll be out the remainder of the
week. But the general direction sounds good.

-- 
Cheers,

David

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

end of thread, other threads:[~2026-07-21 18:12 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 17:00 [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock Rik van Riel
2026-07-17 17:00 ` [PATCH RFC v3 1/6] x86/mm: add untagged_addr_remote_unlocked() Rik van Riel
2026-07-20 11:12   ` Usama Arif
2026-07-17 17:00 ` [PATCH RFC v3 2/6] riscv/mm: " Rik van Riel
2026-07-20 11:57   ` Usama Arif
2026-07-20 15:08     ` Rik van Riel
2026-07-20 16:46       ` Usama Arif
2026-07-20 17:34         ` Rik van Riel
2026-07-20 18:46           ` Usama Arif
2026-07-20 19:21             ` Rik van Riel
2026-07-20 19:39               ` Usama Arif
2026-07-17 17:00 ` [PATCH RFC v3 3/6] mm: rename get_user_page_vma_remote() to get_user_page_lookup_vma() Rik van Riel
2026-07-20 12:00   ` Usama Arif
2026-07-17 17:00 ` [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock Rik van Riel
2026-07-20 12:35   ` Usama Arif
2026-07-20 15:24     ` Rik van Riel
2026-07-17 17:00 ` [PATCH RFC v3 5/6] mm: use per-VMA lock in __access_remote_vm() for single-VMA accesses Rik van Riel
2026-07-17 17:00 ` [PATCH RFC v3 6/6] selftests/mm: cover /proc/pid/mem access to VM_PFNMAP memory Rik van Riel
2026-07-21 18:12 ` [PATCH v3 0/6] mm: access remote process memory under the per-VMA lock David Hildenbrand (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox