All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kasan: fix runtime shadow page table synchronization
@ 2026-10-10 13:37 Muchun Song
  2026-10-16 14:42 ` Andrey Ryabinin
  0 siblings, 1 reply; 3+ messages in thread
From: Muchun Song @ 2026-10-10 13:37 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Andrey Ryabinin
  Cc: Lorenzo Stoakes, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Alexander Potapenko,
	Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Dan Williams,
	linux-mm, kasan-dev, linux-kernel, stable, Muchun Song,
	muchun.song

kasan_populate_early_shadow() is also used after slab becomes available
to install shadow mappings for hot-added memory. When this path
allocates a new P4D or PUD table, p4d_alloc() and pud_alloc() publish it
only in init_mm.

On x86-64, runtime kernel mappings that add a top-level entry must call
arch_sync_kernel_mappings() so existing process page tables can see it.
Without this synchronization, the KASAN shadow mapping can remain
unreachable and fault when the hot-added memory is accessed.

Add p4d_alloc_kernel() and pud_alloc_kernel() to perform the required
synchronization, and use them for runtime KASAN shadow population.

Once copied, a top-level entry remains in other process page tables.
Clearing only init_mm and freeing its target table during teardown would
leave stale pointers behind. Retain these shared levels instead. On x86,
this keeps PUD tables with four-level paging and P4D tables with
five-level paging, while lower-level tables can still be freed.

Fixes: 0207df4fa1a8 ("kernel/memremap, kasan: make ZONE_DEVICE with work with KASAN")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 include/linux/mm.h | 24 ++++++++++++++++++++++++
 mm/kasan/init.c    | 10 ++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5cca90fc1df6..5453d3e42780 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3855,6 +3855,18 @@ static inline p4d_t *p4d_alloc(struct mm_struct *mm, pgd_t *pgd,
 		NULL : p4d_offset(pgd, address);
 }
 
+static inline p4d_t *p4d_alloc_kernel(pgd_t *pgd, unsigned long address)
+{
+	if (unlikely(pgd_none(*pgd))) {
+		if (__p4d_alloc(&init_mm, pgd, address))
+			return NULL;
+		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
+			arch_sync_kernel_mappings(address, address);
+	}
+
+	return p4d_offset(pgd, address);
+}
+
 static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
 		unsigned long address)
 {
@@ -3862,6 +3874,18 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
 		NULL : pud_offset(p4d, address);
 }
 
+static inline pud_t *pud_alloc_kernel(p4d_t *p4d, unsigned long address)
+{
+	if (unlikely(p4d_none(*p4d))) {
+		if (__pud_alloc(&init_mm, p4d, address))
+			return NULL;
+		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
+			arch_sync_kernel_mappings(address, address);
+	}
+
+	return pud_offset(p4d, address);
+}
+
 static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
 {
 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
diff --git a/mm/kasan/init.c b/mm/kasan/init.c
index 66a883887987..93a74a1fcdec 100644
--- a/mm/kasan/init.c
+++ b/mm/kasan/init.c
@@ -206,7 +206,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr,
 			pud_t *p;
 
 			if (slab_is_available()) {
-				p = pud_alloc(&init_mm, p4d, addr);
+				p = pud_alloc_kernel(p4d, addr);
 				if (!p)
 					return -ENOMEM;
 			} else {
@@ -268,7 +268,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
 		if (pgd_none(*pgd)) {
 
 			if (slab_is_available()) {
-				if (!p4d_alloc(&init_mm, pgd, addr))
+				if (!p4d_alloc_kernel(pgd, addr))
 					return -ENOMEM;
 			} else {
 				pgd_populate_kernel(addr, pgd,
@@ -322,6 +322,9 @@ static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d)
 			return;
 	}
 
+	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
+		return;
+
 	pud_free(&init_mm, pud_start);
 	p4d_clear(p4d);
 }
@@ -337,6 +340,9 @@ static void kasan_free_p4d(p4d_t *p4d_start, pgd_t *pgd)
 			return;
 	}
 
+	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
+		return;
+
 	p4d_free(&init_mm, p4d_start);
 	pgd_clear(pgd);
 }

base-commit: 8b38ed9ab5b09c8ba168cbcc49524e9b380ee5c4
-- 
2.54.0



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

* Re: [PATCH] kasan: fix runtime shadow page table synchronization
  2026-10-10 13:37 [PATCH] kasan: fix runtime shadow page table synchronization Muchun Song
@ 2026-10-16 14:42 ` Andrey Ryabinin
  2026-10-17  4:42   ` Muchun Song
  0 siblings, 1 reply; 3+ messages in thread
From: Andrey Ryabinin @ 2026-10-16 14:42 UTC (permalink / raw)
  To: Muchun Song, Andrew Morton, David Hildenbrand
  Cc: Lorenzo Stoakes, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Alexander Potapenko,
	Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Dan Williams,
	linux-mm, kasan-dev, linux-kernel, stable, muchun.song

Muchun Song <songmuchun@bytedance.com> writes:

> kasan_populate_early_shadow() is also used after slab becomes available
> to install shadow mappings for hot-added memory. When this path
> allocates a new P4D or PUD table, p4d_alloc() and pud_alloc() publish it
> only in init_mm.
>
> On x86-64, runtime kernel mappings that add a top-level entry must call
> arch_sync_kernel_mappings() so existing process page tables can see it.
> Without this synchronization, the KASAN shadow mapping can remain
> unreachable and fault when the hot-added memory is accessed.
>

Did you actually hit this crash or it's just LLM's/code inspection finding?

> Add p4d_alloc_kernel() and pud_alloc_kernel() to perform the required
> synchronization, and use them for runtime KASAN shadow population.
>
> Once copied, a top-level entry remains in other process page tables.
> Clearing only init_mm and freeing its target table during teardown would
> leave stale pointers behind. Retain these shared levels instead. On x86,
> this keeps PUD tables with four-level paging and P4D tables with
> five-level paging, while lower-level tables can still be freed.
>
> Fixes: 0207df4fa1a8 ("kernel/memremap, kasan: make ZONE_DEVICE with work with KASAN")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  include/linux/mm.h | 24 ++++++++++++++++++++++++
>  mm/kasan/init.c    | 10 ++++++++--
>  2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5cca90fc1df6..5453d3e42780 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3855,6 +3855,18 @@ static inline p4d_t *p4d_alloc(struct mm_struct *mm, pgd_t *pgd,
>  		NULL : p4d_offset(pgd, address);
>  }
>
> +static inline p4d_t *p4d_alloc_kernel(pgd_t *pgd, unsigned long address)
> +{
> +	if (unlikely(pgd_none(*pgd))) {
> +		if (__p4d_alloc(&init_mm, pgd, address))
> +			return NULL;
> +		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
> +			arch_sync_kernel_mappings(address, address);
> +	}
> +
> +	return p4d_offset(pgd, address);
> +}
> +
>  static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
>  		unsigned long address)
>  {
> @@ -3862,6 +3874,18 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
>  		NULL : pud_offset(p4d, address);
>  }
>
> +static inline pud_t *pud_alloc_kernel(p4d_t *p4d, unsigned long address)
> +{
> +	if (unlikely(p4d_none(*p4d))) {
> +		if (__pud_alloc(&init_mm, p4d, address))
> +			return NULL;
> +		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
> +			arch_sync_kernel_mappings(address, address);
> +	}
> +
> +	return pud_offset(p4d, address);
> +}
> +
>  static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
>  {
>  	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
> index 66a883887987..93a74a1fcdec 100644
> --- a/mm/kasan/init.c
> +++ b/mm/kasan/init.c
> @@ -206,7 +206,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr,
>  			pud_t *p;
>
>  			if (slab_is_available()) {
> -				p = pud_alloc(&init_mm, p4d, addr);
> +				p = pud_alloc_kernel(p4d, addr);

I think it would be better to reuse pud_alloc_track() here.
Pass a pgtbl_mod_mask through zero_p4d/pud/pmd_populate(), and call
arch_sync_kernel_mappings()
only once at the end of kasan_populate_early_shadow()

>  				if (!p)
>  					return -ENOMEM;
>  			} else {
> @@ -268,7 +268,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>  		if (pgd_none(*pgd)) {
>
>  			if (slab_is_available()) {
> -				if (!p4d_alloc(&init_mm, pgd, addr))
> +				if (!p4d_alloc_kernel(pgd, addr))

Ditto

>  					return -ENOMEM;
>  			} else {
>  				pgd_populate_kernel(addr, pgd,
> @@ -322,6 +322,9 @@ static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d)
>  			return;
>  	}
>
> +	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
> +		return;
> +

With 4-level paging the PUD table is now kept here after it has been
synced to every pgd. A later kasan_add_zero_shadow() call can overwrite
it in init_mm only. This might cause a problem, because the aligned
shortcuts in kasan_populate_early_shadow() and zero_p4d_populate()
don't check whether the entry is already populated:

mm/kasan/init.c:kasan_populate_early_shadow() {
    ...
                if (IS_ALIGNED(addr, PGDIR_SIZE) && end - addr >= PGDIR_SIZE) {
            ...
                        p4d_populate_kernel(addr, p4d,
                                        lm_alias(kasan_early_shadow_pud));
    ...
}

So sequence of kasan_add_zero_shadow(), kasan_remove_zero_shadow(),
kasan_add_zero_shadow(), arch_sync_kernel_mappings()
might trigger the BUG_ON() in sync_global_pgds_l4():

arch/x86/mm/init_64.c:sync_global_pgds_l4() {
    ...
                        if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
                                BUG_ON(p4d_pgtable(*p4d)
                                       != p4d_pgtable(*p4d_ref));
    ...
}

Perhaps the aligned populate paths should skip an entry that is
already populated.

>  	pud_free(&init_mm, pud_start);
>  	p4d_clear(p4d);
>  }


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

* Re: [PATCH] kasan: fix runtime shadow page table synchronization
  2026-10-16 14:42 ` Andrey Ryabinin
@ 2026-10-17  4:42   ` Muchun Song
  0 siblings, 0 replies; 3+ messages in thread
From: Muchun Song @ 2026-10-17  4:42 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Muchun Song, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Alexander Potapenko,
	Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Dan Williams,
	linux-mm, kasan-dev, linux-kernel, stable



> On Oct 16, 2026, at 22:42, Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
> 
> Muchun Song <songmuchun@bytedance.com> writes:
> 
>> kasan_populate_early_shadow() is also used after slab becomes available
>> to install shadow mappings for hot-added memory. When this path
>> allocates a new P4D or PUD table, p4d_alloc() and pud_alloc() publish it
>> only in init_mm.
>> 
>> On x86-64, runtime kernel mappings that add a top-level entry must call
>> arch_sync_kernel_mappings() so existing process page tables can see it.
>> Without this synchronization, the KASAN shadow mapping can remain
>> unreachable and fault when the hot-added memory is accessed.
>> 
> 
> Did you actually hit this crash or it's just LLM's/code inspection finding?

Just code inspection finding.

Because my recent work is converting the vmemmap population path to use the
generic pxd_alloc() for page table allocation. I ran into a similar problem,
so I wanted to refer to how KASAN solved it, and then I found the bug.

> 
>> Add p4d_alloc_kernel() and pud_alloc_kernel() to perform the required
>> synchronization, and use them for runtime KASAN shadow population.
>> 
>> Once copied, a top-level entry remains in other process page tables.
>> Clearing only init_mm and freeing its target table during teardown would
>> leave stale pointers behind. Retain these shared levels instead. On x86,
>> this keeps PUD tables with four-level paging and P4D tables with
>> five-level paging, while lower-level tables can still be freed.
>> 
>> Fixes: 0207df4fa1a8 ("kernel/memremap, kasan: make ZONE_DEVICE with work with KASAN")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> include/linux/mm.h | 24 ++++++++++++++++++++++++
>> mm/kasan/init.c    | 10 ++++++++--
>> 2 files changed, 32 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5cca90fc1df6..5453d3e42780 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3855,6 +3855,18 @@ static inline p4d_t *p4d_alloc(struct mm_struct *mm, pgd_t *pgd,
>> 		NULL : p4d_offset(pgd, address);
>> }
>> 
>> +static inline p4d_t *p4d_alloc_kernel(pgd_t *pgd, unsigned long address)
>> +{
>> + 	if (unlikely(pgd_none(*pgd))) {
>> + 		if (__p4d_alloc(&init_mm, pgd, address))
>> + 			return NULL;
>> + 		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
>> + 			arch_sync_kernel_mappings(address, address);
>> + 	}
>> +
>> + 	return p4d_offset(pgd, address);
>> +}
>> +
>> static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
>> 		unsigned long address)
>> {
>> @@ -3862,6 +3874,18 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
>> 		NULL : pud_offset(p4d, address);
>> }
>> 
>> +static inline pud_t *pud_alloc_kernel(p4d_t *p4d, unsigned long address)
>> +{
>> + 	if (unlikely(p4d_none(*p4d))) {
>> + 		if (__pud_alloc(&init_mm, p4d, address))
>> + 			return NULL;
>> + 		if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
>> + 			arch_sync_kernel_mappings(address, address);
>> + 	}
>> +
>> + 	return pud_offset(p4d, address);
>> +}
>> +
>> static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
>> {
>> 	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
>> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
>> index 66a883887987..93a74a1fcdec 100644
>> --- a/mm/kasan/init.c
>> +++ b/mm/kasan/init.c
>> @@ -206,7 +206,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr,
>> 			pud_t *p;
>> 
>> 			if (slab_is_available()) {
>> - 				p = pud_alloc(&init_mm, p4d, addr);
>> + 				p = pud_alloc_kernel(p4d, addr);
> 
> I think it would be better to reuse pud_alloc_track() here.
> Pass a pgtbl_mod_mask through zero_p4d/pud/pmd_populate(), and call
> arch_sync_kernel_mappings()
> only once at the end of kasan_populate_early_shadow()

OK. No problem.

>> 				if (!p)
>> 					return -ENOMEM;
>> 			} else {
>> @@ -268,7 +268,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>> 		if (pgd_none(*pgd)) {
>> 
>> 			if (slab_is_available()) {
>> - 				if (!p4d_alloc(&init_mm, pgd, addr))
>> + 				if (!p4d_alloc_kernel(pgd, addr))
> 
> Ditto

Yeah.

> 
>> 					return -ENOMEM;
>> 			} else {
>> 				pgd_populate_kernel(addr, pgd,
>> @@ -322,6 +322,9 @@ static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d)
>> 			return;
>> 	}
>> 
>> + 	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
>> + 		return;
>> +
> 
> With 4-level paging the PUD table is now kept here after it has been
> synced to every pgd. A later kasan_add_zero_shadow() call can overwrite
> it in init_mm only. This might cause a problem, because the aligned

Good catch.

> shortcuts in kasan_populate_early_shadow() and zero_p4d_populate()
> don't check whether the entry is already populated:
> 
> mm/kasan/init.c:kasan_populate_early_shadow() {
>    ...
>                if (IS_ALIGNED(addr, PGDIR_SIZE) && end - addr >= PGDIR_SIZE) {
>            ...
>                        p4d_populate_kernel(addr, p4d,
>                                        lm_alias(kasan_early_shadow_pud));
>    ...
> }
> 
> So sequence of kasan_add_zero_shadow(), kasan_remove_zero_shadow(),
> kasan_add_zero_shadow(), arch_sync_kernel_mappings()
> might trigger the BUG_ON() in sync_global_pgds_l4():
> 
> arch/x86/mm/init_64.c:sync_global_pgds_l4() {
>    ...
>                        if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
>                                BUG_ON(p4d_pgtable(*p4d)
>                                       != p4d_pgtable(*p4d_ref));
>    ...
> }
> 
> Perhaps the aligned populate paths should skip an entry that is
> already populated.

I think so.

I'll update a new version to fix this.

Thanks,
Muchun

> 
>> 	pud_free(&init_mm, pud_start);
>> 	p4d_clear(p4d);
>> }




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

end of thread, other threads:[~2026-10-17  4:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10 13:37 [PATCH] kasan: fix runtime shadow page table synchronization Muchun Song
2026-10-16 14:42 ` Andrey Ryabinin
2026-10-17  4:42   ` Muchun Song

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.