mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
@ 2026-11-04  9:10 Hui Zhu
  2026-11-05 10:12 ` David Hildenbrand
  2026-11-05 12:00 ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 3+ messages in thread
From: Hui Zhu @ 2026-11-04  9:10 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, linux-kernel
  Cc: Hui Zhu, stable+noautosel

From: Hui Zhu <zhuhui@kylinos.cn>

KCSAN reports a data-race in do_read_fault / pmd_install; the same code
is in mainline:

  write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
   native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
   pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
   pmd_install+0xe0/0x120 mm/memory.c:444
   finish_fault+0x5bc/0xb00 mm/memory.c:5204
   do_cow_fault mm/memory.c:5432 [inline]
   do_fault+0x5d0/0x6e0 mm/memory.c:5526
   handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
   __mm_populate+0x144/0x270 mm/gup.c:2207
   __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764

  read to 0xffff8d52a75fb810 of 8 bytes by task 27527 on cpu 9:
   do_fault_around mm/memory.c:5341 [inline]
   do_read_fault+0x162/0x670 mm/memory.c:5381
   do_fault+0x31a/0x6e0 mm/memory.c:5524
   handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
   faultin_page_range+0x227/0x530 mm/gup.c:2156
   __x64_sys_madvise+0x67/0x80 mm/madvise.c:1479

  value changed: 0x0000000000000000 -> 0x000000001386c067

Both paths only hold mmap_lock for read, so do_fault_around() tests
vmf->pmd while pmd_install() populates it under the pmd lock.  The race is
benign: a stale pmd_none() only wastes the prealloc_pte, which do_fault()
frees again, and a pmd seen as populated only makes ->map_pages() find no
PTE and do_read_fault() fall back to __do_fault().

The load still has to be lockless-safe.  The store side already is: on
i386-PAE, pmd_install() publishes both halves of the pmd with a single
cmpxchg8b.  The load side is not.  pmdp_get() is a plain READ_ONCE() over
a pmd_t that is wider than a word there, so it becomes two 32-bit loads
and can return the low half of one pmd together with the high half of
another.  Use pmdp_get_lockless(), the accessor for pmd reads that do not
hold the pmd lock: it re-reads until both halves come from the same pmd.
It is also what __pte_offset_map() already uses on this very pmd a few
instructions later.

Only X86_32+X86_PAE and SH+X2TLB compile the retry loop, these being the
GUP_GET_PXX_LOW_HIGH selectors that have more than two page table levels.
mips32 with 64-bit physical addresses selects GUP_GET_PXX_LOW_HIGH too,
but is 2-level and so falls back to pmdp_get().  On every other
configuration, x86-64 included, pmdp_get_lockless() is pmdp_get(): the
pmd is loaded in one go, the READ_ONCE() silences the report the same
way, and there is no functional change.

Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: stable+noautosel@kernel.org # KCSAN annotation only, no functional change
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Acked-by: Lorenzo Stoakes <ljs@kernel.org>
---
Changelog:
v4:
Commit message only, the diff is identical to v3.  Spell out the affected
arches and add the lore links for v1-v3 (Lorenzo Stoakes).  Reword the
cmpxchg8b sentence: the store is already atomic, it is the load that tears.
Add the Fixes: tag (David Hildenbrand).
v3:
https://lore.kernel.org/all/20261103014749.96764-1-hui.zhu@linux.dev/
Shorten the comment to one line (David Hildenbrand).
v2:
https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@linux.dev/
Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
v1:
https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@linux.dev/

 mm/memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 0fb676dd2bd63..f25c2ce0ef153 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6017,7 +6017,8 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
 	to_pte = min3(from_pte + nr_pages, (pgoff_t)PTRS_PER_PTE,
 		      pte_off + vma_pages(vmf->vma) - vma_off) - 1;
 
-	if (pmd_none(*vmf->pmd)) {
+	/* pmd_install() rechecks pmd_none() under the pmd lock. */
+	if (pmd_none(pmdp_get_lockless(vmf->pmd))) {
 		vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm);
 		if (!vmf->prealloc_pte)
 			return VM_FAULT_OOM;
-- 
2.43.0


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

* Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
  2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
@ 2026-11-05 10:12 ` David Hildenbrand
  2026-11-05 12:00 ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 3+ messages in thread
From: David Hildenbrand @ 2026-11-05 10:12 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, linux-kernel,
	Hui Zhu
  Cc: Hui Zhu, stable+noautosel

On Wed, 04 Nov 2026 17:10:50 +0800, Hui Zhu wrote:
> KCSAN reports a data-race in do_read_fault / pmd_install; the same code
> is in mainline:
> 
>   write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
>    native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
>    pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
>    pmd_install+0xe0/0x120 mm/memory.c:444
>    finish_fault+0x5bc/0xb00 mm/memory.c:5204
>    do_cow_fault mm/memory.c:5432 [inline]
>    do_fault+0x5d0/0x6e0 mm/memory.c:5526
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    __mm_populate+0x144/0x270 mm/gup.c:2207
>    __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764
> 
> [...]

Applied, thanks!

tree: mm/core.git
branch: core-7.5.misc

Applied patches will appear soon in mm-next, followed by linux-next.

Please report any outstanding bugs that were missed during review, so we
can either drop the patch series or decide how to fix it up.

It is encouraged to provide Acked-bys, Reviewed-bys, and Tested-bys even
though the patches have already been applied. If possible, patch trailers
will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. Further, this is not a guarantee that the
patches will go upstream. If in doubt, please check the listed branch.

[1/1] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
      https://git.kernel.org/mm/core/c/6ab55cfd

-- 
Cheers,

David


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

* Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
  2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
  2026-11-05 10:12 ` David Hildenbrand
@ 2026-11-05 12:00 ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 3+ messages in thread
From: David Hildenbrand (Arm) @ 2026-11-05 12:00 UTC (permalink / raw)
  To: Hui Zhu, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, linux-kernel, Geert Uytterhoeven, Sam Creasey
  Cc: Hui Zhu, stable+noautosel, linux-m68k

On 11/4/26 10:10, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> KCSAN reports a data-race in do_read_fault / pmd_install; the same code
> is in mainline:
> 
>   write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
>    native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
>    pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
>    pmd_install+0xe0/0x120 mm/memory.c:444
>    finish_fault+0x5bc/0xb00 mm/memory.c:5204
>    do_cow_fault mm/memory.c:5432 [inline]
>    do_fault+0x5d0/0x6e0 mm/memory.c:5526
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    __mm_populate+0x144/0x270 mm/gup.c:2207
>    __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764
> 
>   read to 0xffff8d52a75fb810 of 8 bytes by task 27527 on cpu 9:
>    do_fault_around mm/memory.c:5341 [inline]
>    do_read_fault+0x162/0x670 mm/memory.c:5381
>    do_fault+0x31a/0x6e0 mm/memory.c:5524
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    faultin_page_range+0x227/0x530 mm/gup.c:2156
>    __x64_sys_madvise+0x67/0x80 mm/madvise.c:1479
> 
>   value changed: 0x0000000000000000 -> 0x000000001386c067
> 
> Both paths only hold mmap_lock for read, so do_fault_around() tests
> vmf->pmd while pmd_install() populates it under the pmd lock.  The race is
> benign: a stale pmd_none() only wastes the prealloc_pte, which do_fault()
> frees again, and a pmd seen as populated only makes ->map_pages() find no
> PTE and do_read_fault() fall back to __do_fault().
> 
> The load still has to be lockless-safe.  The store side already is: on
> i386-PAE, pmd_install() publishes both halves of the pmd with a single
> cmpxchg8b.  The load side is not.  pmdp_get() is a plain READ_ONCE() over
> a pmd_t that is wider than a word there, so it becomes two 32-bit loads
> and can return the low half of one pmd together with the high half of
> another.  Use pmdp_get_lockless(), the accessor for pmd reads that do not
> hold the pmd lock: it re-reads until both halves come from the same pmd.
> It is also what __pte_offset_map() already uses on this very pmd a few
> instructions later.
> 
> Only X86_32+X86_PAE and SH+X2TLB compile the retry loop, these being the
> GUP_GET_PXX_LOW_HIGH selectors that have more than two page table levels.
> mips32 with 64-bit physical addresses selects GUP_GET_PXX_LOW_HIGH too,
> but is 2-level and so falls back to pmdp_get().  On every other
> configuration, x86-64 included, pmdp_get_lockless() is pmdp_get(): the
> pmd is loaded in one go, the READ_ONCE() silences the report the same
> way, and there is no functional change.
> 
> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: stable+noautosel@kernel.org # KCSAN annotation only, no functional change
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> Changelog:
> v4:
> Commit message only, the diff is identical to v3.  Spell out the affected
> arches and add the lore links for v1-v3 (Lorenzo Stoakes).  Reword the
> cmpxchg8b sentence: the store is already atomic, it is the load that tears.
> Add the Fixes: tag (David Hildenbrand).
> v3:
> https://lore.kernel.org/all/20261103014749.96764-1-hui.zhu@linux.dev/
> Shorten the comment to one line (David Hildenbrand).
> v2:
> https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@linux.dev/
> Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
> v1:
> https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@linux.dev/
> 
>  mm/memory.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 0fb676dd2bd63..f25c2ce0ef153 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6017,7 +6017,8 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
>  	to_pte = min3(from_pte + nr_pages, (pgoff_t)PTRS_PER_PTE,
>  		      pte_off + vma_pages(vmf->vma) - vma_off) - 1;
>  
> -	if (pmd_none(*vmf->pmd)) {
> +	/* pmd_install() rechecks pmd_none() under the pmd lock. */
> +	if (pmd_none(pmdp_get_lockless(vmf->pmd))) {
>  		vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm);
>  		if (!vmf->prealloc_pte)
>  			return VM_FAULT_OOM;

Fun, this breaks some odd m68k configs because they do some legacy weird stuff with pmd_none().

I'll throw in the following cleanup. CCing m68k maintainers: I would drag
this along with this fix here unless you disagree.

From e3c739933a04743e3923f6f39a603e93f2700304 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Thu, 5 Nov 2026 12:49:38 +0100
Subject: [PATCH] m68k/mm: simplify
 pmd_none()/pmd_val()/pmd_present()/pmd_bad()

pmd_t on m68k used to contain 16 words. Commit ef22d8abd876 ("m68k:
mm: Restructure Motorola MMU page-table layout") reduced Motorola PMDs
to one word; Sun3 and ColdFire use word-sized folded PMDs.

Use direct member access in pmd_val() and replace the Sun3 and ColdFire
pointer-taking predicate wrappers with inline functions taking values.

This allows us to write something like:

  if (pmd_none(pmdp_get(pmdp)))

While at it remove unused pmd_present2() and commented-out pmd_bad()
on sun3.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 arch/m68k/include/asm/mcf_pgtable.h  | 20 +++++++++++++++-----
 arch/m68k/include/asm/page.h         |  2 +-
 arch/m68k/include/asm/sun3_pgtable.h | 23 +++++++++++++++--------
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/m68k/include/asm/mcf_pgtable.h b/arch/m68k/include/asm/mcf_pgtable.h
index f45a882238dbb..4efa8e3e1e98a 100644
--- a/arch/m68k/include/asm/mcf_pgtable.h
+++ b/arch/m68k/include/asm/mcf_pgtable.h
@@ -130,11 +130,21 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr,
 
 #define pte_page(pte)	virt_to_page(__pte_page(pte))
 
-static inline int pmd_none2(pmd_t *pmd) { return !pmd_val(*pmd); }
-#define pmd_none(pmd) pmd_none2(&(pmd))
-static inline int pmd_bad2(pmd_t *pmd) { return 0; }
-#define pmd_bad(pmd) pmd_bad2(&(pmd))
-#define pmd_present(pmd) (!pmd_none2(&(pmd)))
+static inline int pmd_none(pmd_t pmd)
+{
+	return !pmd_val(pmd);
+}
+
+static inline int pmd_bad(pmd_t pmd)
+{
+	return 0;
+}
+
+static inline int pmd_present(pmd_t pmd)
+{
+	return !pmd_none(pmd);
+}
+
 static inline void pmd_clear(pmd_t *pmdp) { pmd_val(*pmdp) = 0; }
 
 
diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
index d30f8b2f15927..09a2c020ec992 100644
--- a/arch/m68k/include/asm/page.h
+++ b/arch/m68k/include/asm/page.h
@@ -17,7 +17,7 @@
  */
 #if !defined(CONFIG_MMU) || CONFIG_PGTABLE_LEVELS == 3
 typedef struct { unsigned long pmd; } pmd_t;
-#define pmd_val(x)	((&x)->pmd)
+#define pmd_val(x)	((x).pmd)
 #define __pmd(x)	((pmd_t) { (x) } )
 #endif
 
diff --git a/arch/m68k/include/asm/sun3_pgtable.h b/arch/m68k/include/asm/sun3_pgtable.h
index 704442a391fd7..5867d24032655 100644
--- a/arch/m68k/include/asm/sun3_pgtable.h
+++ b/arch/m68k/include/asm/sun3_pgtable.h
@@ -109,14 +109,21 @@ static inline void pte_clear (struct mm_struct *mm, unsigned long addr, pte_t *p
 #define pmd_page(pmd)		virt_to_page((void *)pmd_page_vaddr(pmd))
 
 
-static inline int pmd_none2 (pmd_t *pmd) { return !pmd_val (*pmd); }
-#define pmd_none(pmd) pmd_none2(&(pmd))
-//static inline int pmd_bad (pmd_t pmd) { return (pmd_val (pmd) & SUN3_PMD_MASK) != SUN3_PMD_MAGIC; }
-static inline int pmd_bad2 (pmd_t *pmd) { return 0; }
-#define pmd_bad(pmd) pmd_bad2(&(pmd))
-static inline int pmd_present2 (pmd_t *pmd) { return pmd_val (*pmd) & SUN3_PMD_VALID; }
-/* #define pmd_present(pmd) pmd_present2(&(pmd)) */
-#define pmd_present(pmd) (!pmd_none2(&(pmd)))
+static inline int pmd_none(pmd_t pmd)
+{
+	return !pmd_val(pmd);
+}
+
+static inline int pmd_bad(pmd_t pmd)
+{
+	return 0;
+}
+
+static inline int pmd_present(pmd_t pmd)
+{
+	return !pmd_none(pmd);
+}
+
 static inline void pmd_clear (pmd_t *pmdp) { pmd_val (*pmdp) = 0; }
 
 /*
-- 
2.43.0


-- 
Cheers,

David

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

end of thread, other threads:[~2026-11-05 12:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
2026-11-05 10:12 ` David Hildenbrand
2026-11-05 12:00 ` 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

all inboxes | Powered by JetHome®