All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5 mm-hotfixes 0/3] mm, x86: fix crash due to missing page table sync and make it harder to miss
@ 2025-08-18  2:02 Harry Yoo
  2025-08-18  2:02 ` [PATCH V5 mm-hotfixes 1/3] mm: move page table sync declarations to linux/pgtable.h Harry Yoo
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Harry Yoo @ 2025-08-18  2:02 UTC (permalink / raw)
  To: Andrew Morton, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, Andy Lutomirski, Peter Zijlstra, H . Peter Anvin
  Cc: Andrey Ryabinin, Arnd Bergmann, Dennis Zhou, Tejun Heo,
	Christoph Lameter, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Vincenzo Frascino, Juergen Gross, Kevin Brodsky,
	Oscar Salvador, Joao Martins, Lorenzo Sccakes, Jane Chu,
	Alistair Popple, Mike Rapoport, David Hildenbrand,
	Gwan-gyeong Mun, Aneesh Kumar K . V, Uladzislau Rezki,
	Liam R . Howlett, Vlastimil Babka, Suren Baghdasaryan,
	Michal Hocko, Qi Zheng, Ard Biesheuvel, Thomas Huth, John Hubbard,
	Ryan Roberts, Peter Xu, Dev Jain, Bibo Mao, Anshuman Khandual,
	Joerg Roedel, x86, linux-kernel, linux-arch, linux-mm, Harry Yoo

To x86 folks:
It's not clear whether this should go through the MM tree or the x86
tree as it changes both. We could send it to the MM tree with Acks
from the x86 folks, or we could send it through the x86 tree instead.
What do you think?

This patch series includes only minimal changes necessary for
backporting the fix to -stable. Planned follow-up patches:
- treewide: include linux/pgalloc.h instead of asm/pgalloc.h
  in common code
- MAINTAINERS: add include/linux/pgalloc.h to MM CORE
- x86/mm/64: convert p*d_populate{,_init} to _kernel variants
- x86/mm/64: drop unnecessary calls to sync_global_pgds() and
  fold it into its sole user

v4 -> v5 (Only cosmetic changes in comments and commit messages):
- Updated comment in PGTBL_*_MODIFIED that I missed in the last version. 
- Added Acked-by, Reviewed-by tags (thanks Kiryl, Mike, Lorenzo and Ulad!)
- Updated commit messages of patch 2 and 3 (Lorenzo)
- Added a comment in arch_sync_kernel_mappings() (Lorenzo)
- Rebased onto the latest mm-hotfixes-unstable (e4321cf73f53)

$ git range-diff \
  rework-sync-kernel-pagetables-v4~3..rework-sync-kernel-pagetables-v4 \
  rework-sync-kernel-pagetables-v5~3..rework-sync-kernel-pagetables-v5

1:  003cb4e564d9 ! 1:  79c2286f7739 mm: move page table sync declarations to linux/pgtable.h
    @@ Commit message
     
         Cc: <stable@vger.kernel.org>
         Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
    +    Acked-by: Kiryl Shutsemau <kas@kernel.org>
    +    Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
    +    Reviewed-by: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
    +    Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
         Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
     
      ## include/linux/pgtable.h ##
2:  7493cc93874d ! 2:  c3c34f3ed699 mm: introduce and use {pgd,p4d}_populate_kernel()
    @@ Commit message
         and the actual synchronization is performed by arch_sync_kernel_mappings().
     
         This change currently targets only x86_64, so only PGD and P4D level
    -    helpers are introduced. In theory, PUD and PMD level helpers can be added
    -    later if needed by other architectures.
    +    helpers are introduced. Currently, these helpers are no-ops since no
    +    architecture sets PGTBL_{PGD,P4D}_MODIFIED in ARCH_PAGE_TABLE_SYNC_MASK.
     
    -    Currently this is a no-op, since no architecture sets
    -    PGTBL_{PGD,P4D}_MODIFIED in ARCH_PAGE_TABLE_SYNC_MASK.
    +    In theory, PUD and PMD level helpers can be added later if needed by
    +    other architectures. For now, 32-bit architectures (x86-32 and arm) only
    +    handle PGTBL_PMD_MODIFIED, so p*d_populate_kernel() will never affect
    +    them unless we introduce a PMD level helper.
     
         Cc: <stable@vger.kernel.org>
         Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
         Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
    +    Acked-by: Kiryl Shutsemau <kas@kernel.org>
    +    Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
    +    Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
         Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
     
      ## include/linux/pgalloc.h (new) ##
    @@ include/linux/pgtable.h: static inline void modify_prot_commit_ptes(struct vm_ar
       */
      #ifndef ARCH_PAGE_TABLE_SYNC_MASK
      #define ARCH_PAGE_TABLE_SYNC_MASK 0
    +@@ include/linux/pgtable.h: static inline bool arch_has_pfn_modify_check(void)
    + /*
    +  * Page Table Modification bits for pgtbl_mod_mask.
    +  *
    +- * These are used by the p?d_alloc_track*() set of functions an in the generic
    +- * vmalloc/ioremap code to track at which page-table levels entries have been
    +- * modified. Based on that the code can better decide when vmalloc and ioremap
    +- * mapping changes need to be synchronized to other page-tables in the system.
    ++ * These are used by the p?d_alloc_track*() and p*d_populate_kernel()
    ++ * functions in the generic vmalloc, ioremap and page table update code
    ++ * to track at which page-table levels entries have been modified.
    ++ * Based on that the code can better decide when page table changes need
    ++ * to be synchronized to other page-tables in the system.
    +  */
    + #define		__PGTBL_PGD_MODIFIED	0
    + #define		__PGTBL_P4D_MODIFIED	1
     
      ## mm/kasan/init.c ##
     @@
3:  f199138e40e6 ! 3:  4ad9ce5c09c2 x86/mm/64: define ARCH_PAGE_TABLE_SYNC_MASK and arch_sync_kernel_mappings()
    @@ Commit message
         x86/mm/64: define ARCH_PAGE_TABLE_SYNC_MASK and arch_sync_kernel_mappings()
     
         Define ARCH_PAGE_TABLE_SYNC_MASK and arch_sync_kernel_mappings() to ensure
    -    page tables are properly synchronized when calling p*d_populate_kernel().
    -    It is inteneded to synchronize page tables via pgd_pouplate_kernel() when
    -    5-level paging is in use and via p4d_pouplate_kernel() when 4-level paging
    -    is used.
    +    page tables are properly synchronized when calling
    +    p*d_populate_kernel().
    +
    +    For 5-level paging, synchronization is performed via
    +    pgd_populate_kernel(). In 4-level paging, pgd_populate() is a no-op,
    +    so synchronization is instead performed at the P4D level via
    +    p4d_populate_kernel().
     
         This fixes intermittent boot failures on systems using 4-level paging
         and a large amount of persistent memory:
    @@ Commit message
         Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges")
         Closes: https://lore.kernel.org/linux-mm/20250311114420.240341-1-gwan-gyeong.mun@intel.com [1]
         Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
    +    Acked-by: Kiryl Shutsemau <kas@kernel.org>
    +    Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
    +    Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
         Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
     
      ## arch/x86/include/asm/pgtable_64_types.h ##
    @@ arch/x86/mm/init_64.c: static void sync_global_pgds(unsigned long start, unsigne
      		sync_global_pgds_l4(start, end);
      }
      
    ++/*
    ++ * Make kernel mappings visible in all page tables in the system.
    ++ * This is necessary except when the init task populates kernel mappings
    ++ * during the boot process. In that case, all processes originating from
    ++ * the init task copies the kernel mappings, so there is no issue.
    ++ * Otherwise, missing synchronization could lead to kernel crashes due
    ++ * to missing page table entries for certain kernel mappings.
    ++ *
    ++ * Synchronization is performed at the top level, which is the PGD in
    ++ * 5-level paging systems. But in 4-level paging systems, however,
    ++ * pgd_populate() is a no-op, so synchronization is done at the P4D level.
    ++ * sync_global_pgds() handles this difference between paging levels.
    ++ */
     +void arch_sync_kernel_mappings(unsigned long start, unsigned long end)
     +{
     +	sync_global_pgds(start, end);

[ The actual cover letter starts here ]

# The problem: It is easy to miss/overlook page table synchronization

Hi all,

During our internal testing, we started observing intermittent boot
failures when the machine uses 4-level paging and has a large amount
of persistent memory:

  BUG: unable to handle page fault for address: ffffe70000000034
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0002) - not-present page
  PGD 0 P4D 0 
  Oops: 0002 [#1] SMP NOPTI
  RIP: 0010:__init_single_page+0x9/0x6d
  Call Trace:
   <TASK>
   __init_zone_device_page+0x17/0x5d
   memmap_init_zone_device+0x154/0x1bb
   pagemap_range+0x2e0/0x40f
   memremap_pages+0x10b/0x2f0
   devm_memremap_pages+0x1e/0x60
   dev_dax_probe+0xce/0x2ec [device_dax]
   dax_bus_probe+0x6d/0xc9
   [... snip ...]
   </TASK>

It turns out that the kernel panics while initializing vmemmap
(struct page array) when the vmemmap region spans two PGD entries,
because the new PGD entry is only installed in init_mm.pgd,
but not in the page tables of other tasks.

And looking at __populate_section_memmap():
  if (vmemmap_can_optimize(altmap, pgmap))                                
          // does not sync top level page tables
          r = vmemmap_populate_compound_pages(pfn, start, end, nid, pgmap);
  else                                                                    
          // sync top level page tables in x86
          r = vmemmap_populate(start, end, nid, altmap);

In the normal path, vmemmap_populate() in arch/x86/mm/init_64.c
synchronizes the top level page table (See commit 9b861528a801
("x86-64, mem: Update all PGDs for direct mapping and vmemmap mapping
changes")) so that all tasks in the system can see the new vmemmap area.

However, when vmemmap_can_optimize() returns true, the optimized path
skips synchronization of top-level page tables. This is because
vmemmap_populate_compound_pages() is implemented in core MM code, which
does not handle synchronization of the top-level page tables. Instead,
the core MM has historically relied on each architecture to perform this
synchronization manually.

We're not the first party to encounter a crash caused by not-sync'd
top level page tables: earlier this year, Gwan-gyeong Mun attempted to
address the issue [1] [2] after hitting a kernel panic when x86 code
accessed the vmemmap area before the corresponding top-level entries
were synced. At that time, the issue was believed to be triggered
only when struct page was enlarged for debugging purposes, and the patch
did not get further updates.

It turns out that current approach of relying on each arch to handle
the page table sync manually is fragile because 1) it's easy to forget
to sync the top level page table, and 2) it's also easy to overlook that
the kernel should not access the vmemmap and direct mapping areas before
the sync.

# The solution: Make page table sync more code robust and harder to miss

To address this, Dave Hansen suggested [3] [4] introducing
{pgd,p4d}_populate_kernel() for updating kernel portion
of the page tables and allow each architecture to explicitly perform
synchronization when installing top-level entries. With this approach,
we no longer need to worry about missing the sync step, reducing the risk
of future regressions.

The new interface reuses existing ARCH_PAGE_TABLE_SYNC_MASK,
PGTBL_P*D_MODIFIED and arch_sync_kernel_mappings() facility used by
vmalloc and ioremap to synchronize page tables.

pgd_populate_kernel() looks like this:
static inline void pgd_populate_kernel(unsigned long addr, pgd_t *pgd,
                                       p4d_t *p4d)
{
        pgd_populate(&init_mm, pgd, p4d);
        if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
                arch_sync_kernel_mappings(addr, addr);
}

It is worth noting that vmalloc() and apply_to_range() carefully
synchronizes page tables by calling p*d_alloc_track() and
arch_sync_kernel_mappings(), and thus they are not affected by
this patch series.

This patch series was hugely inspired by Dave Hansen's suggestion and
hence added Suggested-by: Dave Hansen.

Cc stable because lack of this series opens the door to intermittent
boot failures.

[1] https://lore.kernel.org/linux-mm/20250220064105.808339-1-gwan-gyeong.mun@intel.com
[2] https://lore.kernel.org/linux-mm/20250311114420.240341-1-gwan-gyeong.mun@intel.com
[3] https://lore.kernel.org/linux-mm/d1da214c-53d3-45ac-a8b6-51821c5416e4@intel.com
[4] https://lore.kernel.org/linux-mm/4d800744-7b88-41aa-9979-b245e8bf794b@intel.com 

Harry Yoo (3):
  mm: move page table sync declarations to linux/pgtable.h
  mm: introduce and use {pgd,p4d}_populate_kernel()
  x86/mm/64: define ARCH_PAGE_TABLE_SYNC_MASK and
    arch_sync_kernel_mappings()

 arch/x86/include/asm/pgtable_64_types.h |  3 +++
 arch/x86/mm/init_64.c                   | 18 ++++++++++++++++++
 include/linux/pgalloc.h                 | 24 ++++++++++++++++++++++++
 include/linux/pgtable.h                 | 25 +++++++++++++++++++++----
 include/linux/vmalloc.h                 | 16 ----------------
 mm/kasan/init.c                         | 12 ++++++------
 mm/percpu.c                             |  6 +++---
 mm/sparse-vmemmap.c                     |  6 +++---
 8 files changed, 78 insertions(+), 32 deletions(-)
 create mode 100644 include/linux/pgalloc.h

-- 
2.43.0


^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-08-27  6:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18  2:02 [PATCH V5 mm-hotfixes 0/3] mm, x86: fix crash due to missing page table sync and make it harder to miss Harry Yoo
2025-08-18  2:02 ` [PATCH V5 mm-hotfixes 1/3] mm: move page table sync declarations to linux/pgtable.h Harry Yoo
2025-08-18  7:47   ` David Hildenbrand
2025-08-18  2:02 ` [PATCH V5 mm-hotfixes 2/3] mm: introduce and use {pgd,p4d}_populate_kernel() Harry Yoo
2025-08-18  7:48   ` David Hildenbrand
2025-08-18  9:02   ` kernel test robot
2025-08-18 10:41     ` Harry Yoo
2025-08-21  9:35   ` [PATCH] mm: fix KASAN build error due to p*d_populate_kernel() Harry Yoo
2025-08-21  9:38     ` kernel test robot
2025-08-21 10:10     ` Lorenzo Stoakes
2025-08-21 10:42       ` Harry Yoo
2025-08-21 11:46         ` Lorenzo Stoakes
2025-08-21 11:57     ` [PATCH v2] " Harry Yoo
2025-08-21 12:00       ` kernel test robot
2025-08-21 17:36       ` Dave Hansen
2025-08-22  1:11         ` Harry Yoo
2025-08-22 16:02           ` Andrey Ryabinin
2025-08-27  6:30             ` Harry Yoo
2025-08-22 17:08           ` Dave Hansen
2025-08-25  9:46             ` Andrey Ryabinin
2025-08-22  2:07   ` [PATCH v3] " Harry Yoo
2025-08-18  2:02 ` [PATCH V5 mm-hotfixes 3/3] x86/mm/64: define ARCH_PAGE_TABLE_SYNC_MASK and arch_sync_kernel_mappings() Harry Yoo
2025-08-18  7:49   ` David Hildenbrand
2025-08-18  7:50 ` [PATCH V5 mm-hotfixes 0/3] mm, x86: fix crash due to missing page table sync and make it harder to miss David Hildenbrand

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.