mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race
@ 2026-08-22 17:46 Lorenzo Stoakes (ARM)
  2026-08-22 17:46 ` [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 " Lorenzo Stoakes (ARM)
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-22 17:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Christoffer Dall
  Cc: Wei-Lin Chang, Yao Yuan, linux-arm-kernel, kvmarm, linux-kernel,
	Lorenzo Stoakes (ARM),
	stable

When GFNs are invalidated in L0 an MMU notifier triggers
kvm_unmap_gfn_range() which tears down all of the stage 2 shadow page
tables for nested guests via kvm_nested_s2_unmap().

To avoid lockup, the kvm->mmu_lock is dropped while doing this and the task
rescheduled once for each block of physical address space (32 MiB for 16
KiB page size), with the lock being reacquired once the task is scheduled
again.

This results in a potential race between this L0 tear down and tear down of
the guest itself in kvm_flush_shadow_all(), a race which has been observed
on real hardware.

When this race occurs it causes an invalid kernel warning when the PGT of a
nested MMU is cleared by kvm_flush_shadow_all() ->
kvm_arch_flush_shadow_all() -> kvm_free_stage2_pgd().

Patch 1 fixes this by having stage2_apply_range() no longer return an error
when it has experienced a benign race with pgt teardown when it drops the
lock.

Patch 2 addresses something more serious - bad timing can turn this spurious
warning into a NULL pointer dereference.

kvm_arch_flush_shadow_all() calls kvm_uninit_stage2_mmu() which calls
kvm_free_stage2_pgd() on the canonical kvm->arch.mmu for that guest's S2
mappings, making it NULL.

This is problematic if it happens before stage2_apply_range() reacquires
the kvm->mmu_lock, as it ultimately returns to kvm_nested_s2_unmap() which
dereferences kvm->arch.mmu.pgt with the mmu lock held under the incorrect
assumption that it means it's valid, resulting in a NULL pointer
dereference.

Fix that by checking if kvm->arch.mmu.pgt is NULL before dereferencing it
in kvm_nested_s2_unmap() and kvm_nested_s2_wp().

v2:
* Rebased onto next
* Updated 1/2's commit message to say that it was all of the kvmtool hosts
  that were stopped, as per discussion with Wei-Lin and Yao Yuan.
* Updated 2/2 to remove the !may_block WARN_ON() as duplicative, as per
  Marc.
* Updated 2/2 to abstract the VNCR IPA invalidation in
  kvm_invalidate_vncr_ipa_all() and perform the same check for
  kvm_nested_s2_wp(), as per discussion with Marc and sashiko report.

v1:
https://lore.kernel.org/r/20260812-kvm-arm-nested-virt-fix-v1-0-4ad883f1b6a5@kernel.org

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Lorenzo Stoakes (ARM) (2):
      KVM: arm64: Fix spurious warning for benign stage 2 teardown race
      KVM: arm64: nv: Fix null ptr deref on nested wp/unmap, teardown race

 arch/arm64/kvm/mmu.c    | 10 ++++++++--
 arch/arm64/kvm/nested.c | 15 +++++++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)
---
base-commit: aa8e5dc6a7a2a1141ab40706a51010adcd0e57d2
change-id: 20260811-kvm-arm-nested-virt-fix-031e9ab1be87

Best regards,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>


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

* [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 teardown race
  2026-08-22 17:46 [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race Lorenzo Stoakes (ARM)
@ 2026-08-22 17:46 ` Lorenzo Stoakes (ARM)
  2026-08-23  5:38   ` Yao Yuan
  2026-08-22 17:46 ` [PATCH v2 2/2] KVM: arm64: nv: Fix null ptr deref on nested wp/unmap, " Lorenzo Stoakes (ARM)
  2026-08-23  7:53 ` [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 " Marc Zyngier
  2 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-22 17:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Christoffer Dall
  Cc: Wei-Lin Chang, Yao Yuan, linux-arm-kernel, kvmarm, linux-kernel,
	Lorenzo Stoakes (ARM),
	stable

A batch of kernel warnings were triggered in the L0 host kernel when using
kvmtool to experiment with nested virtualisation.

The issue was observed on an M2 macbook pro with 24 GiB of RAM running
asahi linux 7.1.6-400.asahi.fc44.aarch64+16k with 16 KiB page size.

The bug is confirmed to exist in mainline and does not appear to be
impacted by any downstream patches carried by asahi.

kvmtool was used to establish an L1 guest with 8 CPUs and 8 GiB of RAM.

kvmtool was then run again within the L1 guest to establish an L2 guest
with 4 CPUs and 4 GiB of RAM.

Then it was run again to establish another inner guest with 2 CPUs and 2
GiB of RAM at L3.

Both the L2 and L3 guests were then exited and another L2 guest was
established with the same characteristics.

Sufficient memory pressure was present in the L0 host to trigger indirect
reclaim, waking kcompactd up and triggering migration.

Shortly afterwards all of the kvmtool instances were stopped, at which
point three warnings were observed in dmesg in quick succession on the
host, with the second and the third occurring 114us and 189us after the
first, respectively.

In each case the call trace was the same:

WARNING: arch/arm64/kvm/mmu.c:336 at __unmap_stage2_range+0x64/0x80,
CPU#5: kcompactd0/66
...
__unmap_stage2_range (arch/arm64/kvm/mmu.c:335 (discriminator 3)) (P)
kvm_stage2_unmap_range (arch/arm64/kvm/mmu.c:346)
kvm_nested_s2_unmap (arch/arm64/kvm/nested.c:1168 (discriminator 14))
kvm_unmap_gfn_range (arch/arm64/kvm/mmu.c:2416)
kvm_mmu_notifier_invalidate_range_start (arch/arm64/kvm/../../../virt/kvm/kvm_main.c:718 ...)
mn_hlist_invalidate_range_start (mm/mmu_notifier.c:525)
__mmu_notifier_invalidate_range_start (mm/mmu_notifier.c:580)
try_to_migrate_one (./include/linux/mmu_notifier.h:478 ./include/linux/mmu_notifier.h:471 mm/rmap.c:2459)
rmap_walk_anon (mm/rmap.c:3001)
rmap_walk (mm/rmap.c:3106 mm/rmap.c:3101)
try_to_migrate (mm/rmap.c:2774)
migrate_folio_unmap (mm/migrate.c:1330 (discriminator 3))
migrate_pages_batch (mm/migrate.c:1909)
migrate_pages_sync (mm/migrate.c:2026)
migrate_pages (mm/migrate.c:2135)
compact_zone (mm/compaction.c:2663)
compact_node (mm/compaction.c:2932)
kcompactd (mm/compaction.c:3230)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/arm64/kernel/entry.S:858)

Analysing this:

try_to_migrate() performs the first part of migration on a folio -
establishing migration entries in all page tables mapping it - calling
try_to_migrate_one() for each VMA the folio is mapped in.

Immediately prior to installing the migration entry, try_to_migrate_one()
calls mmu_notifier_invalidate_range_start() to signal to notifiers that the
existing page table entry is about to be unmapped.

Since this range happened to contain folios used by the virtual machine
(given its memory consumption this was highly likely to be a target) this
in turn triggers arm64 kvm code via an MMU notifier:

mmu_notifier_invalidate_range_start()
  -> ... -> kvm_mmu_notifier_invalidate_range_start()
    -> kvm_mmu_unmap_gfn_range()
      -> kvm_unmap_gfn_range()
        -> kvm_nested_s2_unmap()
          -> kvm_stage2_unmap_range()
            -> __unmap_stage2_range()
               -> stage2_apply_range()
       	       <- -EINVAL, triggering a WARN_ON()

Since commit ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page
tables") kvm_unmap_gfn_range() invokes kvm_nested_s2_unmap() which takes
the rather drastic step of evicting the entirety of the stage 2 shadow page
tables for each nested guest:

	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];

		if (kvm_s2_mmu_valid(mmu))
			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
	}

This will iterate through every valid MMU.

mmu_notifier_invalidate_range_start() sets the MMU_NOTIFIER_RANGE_BLOCKABLE
flag when signalling notifiers, so may_block is true here.

__unmap_stage2_range() wraps invocation of stage2_apply_range() in a
WARN_ON():

	WARN_ON(stage2_apply_range(mmu, start, end, KVM_PGT_FN(kvm_pgtable_stage2_unmap),
				   may_block));

Which is precisely the triggered warning and means stage2_apply_range() is
returning an error.

In arm64 return values are stored in the x0 register and in each splat w0
(since it's a 32-bit value) is 0xffffffea which, by two's complement, is
-0b00010110 or -EINVAL.

The only way in which stage2_apply_range() can return an error is if either
the passed in walk function (kvm_pgtable_stage2_unmap()) returns an error
or mmu->pgt is NULL:

	static int stage2_apply_range(...)
	{
		do {
			struct kvm_pgtable *pgt = mmu->pgt;
			if (!pgt)
				return -EINVAL;

			next = stage2_range_addr_end(addr, end);
			ret = fn(pgt, addr, next - addr);
			if (ret)
				break;
			if (resched && next != end)
				cond_resched_rwlock_write(&kvm->mmu_lock);
		} while (addr = next, addr != end);

		return ret;
	}

This loop is batched by stage2_range_addr_end() at the granularity of
kvm_granule_size(KVM_PGTABLE_MIN_BLOCK_LEVEL), which for a 16 KiB page size
kernel is 32 MiB.

kvm_pgtable_stage2_unmap() only returns an error if
kvm_pgtable_walk_begin() or _kvm_pgtable_walk() return an error. On arm64
the former doesn't ever do so, and the latter only returns -EINVAL if
either pgd is NULL (we gate that already) or an invalid pgt->start_level is
specified (not the case).

So the cause of the warning is that mmu->pgt is NULL here.

This field is protected by kvm->mmu_lock, but after each invocation of the
walk function, if the end of the range has not yet been reached, the lock
is dropped.

This is done by calling cond_resched_rwlock_write() which drops
kvm->mmu_lock when yielding the time slice before reacquiring it upon being
scheduled again:

		if (resched && next != end)
			cond_resched_rwlock_write(&kvm->mmu_lock);

Note that resched = may_block here and is always true for these walks.

This points to something else racing this code to clearing the pgt, and
brings us back to the observation above that the issue occurs when tearing
down the L2 guest.

Upon L0's userspace mm_struct teardown, stage 2 mappings for IPAs mapped by
the guest are unmapped via mmu notifier:

exit_mm()
  -> mmput()
    -> __mmput()
      -> exit_mmap()
        -> mmu_notifier_release()
	  -> ... -> kvm_mmu_notifier_release()
            -> kvm_flush_shadow_all()
              -> kvm_arch_flush_shadow_all()
	        -> kvm_free_stage2_pgd()
		  -> [ acquire kvm->mmu_lock for write ]
		  -> mmu->pgt = NULL [ among other tasks ]
		  -> [ release kvm->mmu_lock for write ]

So there is a benign race resulting in a spurious warning:

	Thread A - notify: migration   | Thread B - notify: release
	-------------------------------|---------------------------------
	< kvm->mmu_lock held >         |
	stage2_apply_range()           |
	  get mmu->pgt, check !NULL    |
	  ...                          | kvm_arch_flush_shadow_all()
	  cond_resched_rwlock_write(); |   < contend, sleep kvm->mmu_lock >
	< drop kvm->mmu_lock >         |   < acquire kvm->mmu_lock>
                                       |   ...
				       |   kvm_free_stage2_pgd()
                                       |     mmu->pgt = NULL
				       |   < invalidate MMU >
				       |   ...
				       |   < release kvm->mmu_lock >
	[ scheduled ]		       |
	stage2_apply_range()           |
	  < loop to next >             |
	  get, mmu->pgt, check !NULL   |
	  is NULL, return -EINVAL      |
        __unmap_stage2_range()         |
	  WARN_ON(-EINVAL) <--- entirely spurious - the race was handled
                                 correctly.

This does not hide any other errors as the only other error that can arise
is from kvm_pgtable_stage2_unmap() which calls kvm_pgtable_walk()
first which gates on !NULL pgt->pgd returning -EINVAL if it is NULL anyway.

On reflection this race seems rather likely when there is host memory
pressure concurrent with nested guest teardown - the lock release will
cause interleaving warnings once for every valid MMU as long as at least 1
block remains to be unmapped by stage2_apply_range().

Fix the spurious warning by updating stage2_apply_range() to no longer
treat concurrent PGT teardown on lock release as an error - whether the
walker is tearing down page tables or doing something else this is a
legitimate reason to abort the operation without error.

This keeps the warning in place for all other circumstances.

In practice only __unmap_stage2_range() actually does anything with the
error so this only impacts that.

Fixes: ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page tables")
Cc: stable@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 arch/arm64/kvm/mmu.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 74e7e7f7564c..31e049ded093 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -59,19 +59,25 @@ static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
  * long will also starve other vCPUs. We have to also make sure that the page
  * tables are not freed while we released the lock.
  */
-static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
+static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
 			      phys_addr_t end,
 			      int (*fn)(struct kvm_pgtable *, u64, u64),
 			      bool resched)
 {
 	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
+	phys_addr_t addr = start;
 	int ret;
 	u64 next;
 
 	do {
 		struct kvm_pgtable *pgt = mmu->pgt;
+		/*
+		 * We may be raced on PGT teardown when we release the
+		 * kvm->mmu_lock. That's fine as the PGT is legitimately no
+		 * longer present.
+		 */
 		if (!pgt)
-			return -EINVAL;
+			return resched && addr > start ? 0 : -EINVAL;
 
 		next = stage2_range_addr_end(addr, end);
 		ret = fn(pgt, addr, next - addr);

-- 
2.55.0


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

* [PATCH v2 2/2] KVM: arm64: nv: Fix null ptr deref on nested wp/unmap, teardown race
  2026-08-22 17:46 [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race Lorenzo Stoakes (ARM)
  2026-08-22 17:46 ` [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 " Lorenzo Stoakes (ARM)
@ 2026-08-22 17:46 ` Lorenzo Stoakes (ARM)
  2026-08-23  7:53 ` [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 " Marc Zyngier
  2 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-22 17:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Christoffer Dall
  Cc: Wei-Lin Chang, Yao Yuan, linux-arm-kernel, kvmarm, linux-kernel,
	Lorenzo Stoakes (ARM),
	stable

Commit 7270cc9157f4 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU
notifiers") introduced VNCR_EL2 invalidation in both kvm_nested_s2_unmap()
and kvm_nested_s2_wp().

However at the point of this being performed concurrent stage 2 teardown of
a nested guest can cause kvm->arch.mmu.pgt to be set to NULL.

This happens in kvm_flush_shadow_all() -> kvm_arch_flush_shadow_all() ->
kvm_free_stage2_pgd() and is performed under the kvm->mmu_lock.

Commit ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page
tables") introduced the teardown of the entire nested MMU range, which then
invokes stage2_apply_range() with resched=true:

mmu_notifier_invalidate_range_start()
  -> ... -> kvm_mmu_notifier_invalidate_range_start()
    -> kvm_mmu_unmap_gfn_range()
      -> kvm_unmap_gfn_range()
        -> kvm_nested_s2_unmap()
          -> kvm_stage2_unmap_range()
            -> __unmap_stage2_range()
                -> stage2_apply_range()

This means that stage2_apply_range() can drop the kvm->mmu_lock and thus
concurrent progress can be made in lockstep with
kvm_arch_flush_shadow_all().

If kvm_arch_flush_shadow_all() advances ahead of stage2_apply_range() and
completes its operation it guarantees a NULL pointer deref.

Since kvm_free_stage2_pgd() is performed under the kvm->mmu_lock this will
either be observed NULL or not and serialised against
kvm_free_stage2_pgd().

Resolve the issue by abstracting the invalidation to a new function,
kvm_invalidate_vncr_ipa_all(), and check that the pgt is non-NULL before
dereferencing it.

Fixes: 7270cc9157f4 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU notifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 arch/arm64/kvm/nested.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 17123f0b6dab..f69722e1592a 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1260,6 +1260,17 @@ void kvm_handle_s1e2_tlbi(struct kvm_vcpu *vcpu, u32 inst, u64 val)
 	invalidate_vncr_va(vcpu->kvm, &scope);
 }
 
+static void kvm_invalidate_vncr_ipa_all(struct kvm *kvm)
+{
+	struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;
+
+	lockdep_assert_held_write(&kvm->mmu_lock);
+
+	/* if the mmu lock was dropped, pgt teardown may have raced. */
+	if (pgt)
+		kvm_invalidate_vncr_ipa(kvm, 0, BIT(pgt->ia_bits));
+}
+
 void kvm_nested_s2_wp(struct kvm *kvm)
 {
 	int i;
@@ -1276,7 +1287,7 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 			kvm_stage2_wp_range(mmu, 0, kvm_phys_size(mmu));
 	}
 
-	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
+	kvm_invalidate_vncr_ipa_all(kvm);
 }
 
 void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
@@ -1295,7 +1306,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
 	}
 
-	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
+	kvm_invalidate_vncr_ipa_all(kvm);
 }
 
 void kvm_nested_s2_flush(struct kvm *kvm)

-- 
2.55.0


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

* Re: [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 teardown race
  2026-08-22 17:46 ` [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 " Lorenzo Stoakes (ARM)
@ 2026-08-23  5:38   ` Yao Yuan
  2026-08-24 13:29     ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Yuan @ 2026-08-23  5:38 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Christoffer Dall, Wei-Lin Chang, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Sat, Aug 22, 2026 at 06:46:53PM +0800, Lorenzo Stoakes (ARM) wrote:
> A batch of kernel warnings were triggered in the L0 host kernel when using
> kvmtool to experiment with nested virtualisation.

...

> Fixes: ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page tables")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  arch/arm64/kvm/mmu.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 74e7e7f7564c..31e049ded093 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -59,19 +59,25 @@ static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
>   * long will also starve other vCPUs. We have to also make sure that the page
>   * tables are not freed while we released the lock.
>   */
> -static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
> +static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
>  			      phys_addr_t end,
>  			      int (*fn)(struct kvm_pgtable *, u64, u64),
>  			      bool resched)
>  {
>  	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> +	phys_addr_t addr = start;
>  	int ret;
>  	u64 next;
>
>  	do {
>  		struct kvm_pgtable *pgt = mmu->pgt;
> +		/*
> +		 * We may be raced on PGT teardown when we release the
> +		 * kvm->mmu_lock. That's fine as the PGT is legitimately no
> +		 * longer present.
> +		 */
>  		if (!pgt)
> -			return -EINVAL;
> +			return resched && addr > start ? 0 : -EINVAL;

Hi,

I can understand the addr checking makes sure only return 0
when the mmu_lock is dropped at least once and get pgt =
NULL, may a variable like drop_lock = true when
cond_resched_rwlock_write(&kvm->mmu_lock) happens can have
better readability, but depends on you and others' opinion.

Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>

>
>  		next = stage2_range_addr_end(addr, end);
>  		ret = fn(pgt, addr, next - addr);
>
> --
> 2.55.0

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

* Re: [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race
  2026-08-22 17:46 [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race Lorenzo Stoakes (ARM)
  2026-08-22 17:46 ` [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 " Lorenzo Stoakes (ARM)
  2026-08-22 17:46 ` [PATCH v2 2/2] KVM: arm64: nv: Fix null ptr deref on nested wp/unmap, " Lorenzo Stoakes (ARM)
@ 2026-08-23  7:53 ` Marc Zyngier
  2026-08-24 13:15   ` Lorenzo Stoakes (ARM)
  2 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2026-08-23  7:53 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Christoffer Dall, Wei-Lin Chang, Yao Yuan, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Sat, 22 Aug 2026 18:46:52 +0100,
"Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> 
> When GFNs are invalidated in L0 an MMU notifier triggers
> kvm_unmap_gfn_range() which tears down all of the stage 2 shadow page
> tables for nested guests via kvm_nested_s2_unmap().
> 
> To avoid lockup, the kvm->mmu_lock is dropped while doing this and the task
> rescheduled once for each block of physical address space (32 MiB for 16
> KiB page size), with the lock being reacquired once the task is scheduled
> again.
> 
> This results in a potential race between this L0 tear down and tear down of
> the guest itself in kvm_flush_shadow_all(), a race which has been observed
> on real hardware.
> 
> When this race occurs it causes an invalid kernel warning when the PGT of a
> nested MMU is cleared by kvm_flush_shadow_all() ->
> kvm_arch_flush_shadow_all() -> kvm_free_stage2_pgd().
> 
> Patch 1 fixes this by having stage2_apply_range() no longer return an error
> when it has experienced a benign race with pgt teardown when it drops the
> lock.
> 
> Patch 2 addresses something more serious - bad timing can turn this spurious
> warning into a NULL pointer dereference.
> 
> kvm_arch_flush_shadow_all() calls kvm_uninit_stage2_mmu() which calls
> kvm_free_stage2_pgd() on the canonical kvm->arch.mmu for that guest's S2
> mappings, making it NULL.
> 
> This is problematic if it happens before stage2_apply_range() reacquires
> the kvm->mmu_lock, as it ultimately returns to kvm_nested_s2_unmap() which
> dereferences kvm->arch.mmu.pgt with the mmu lock held under the incorrect
> assumption that it means it's valid, resulting in a NULL pointer
> dereference.
> 
> Fix that by checking if kvm->arch.mmu.pgt is NULL before dereferencing it
> in kvm_nested_s2_unmap() and kvm_nested_s2_wp().

With the commit message for patch #1 trimmed to something that fits on
a couple of standard terminal screens ;-) :

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Jazz isn't dead. It just smells funny.

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

* Re: [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race
  2026-08-23  7:53 ` [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 " Marc Zyngier
@ 2026-08-24 13:15   ` Lorenzo Stoakes (ARM)
  2026-08-24 15:56     ` Marc Zyngier
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-24 13:15 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Christoffer Dall, Wei-Lin Chang, Yao Yuan, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Sun, Aug 23, 2026 at 08:53:31AM +0100, Marc Zyngier wrote:
> On Sat, 22 Aug 2026 18:46:52 +0100,
> "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> >
> > When GFNs are invalidated in L0 an MMU notifier triggers
> > kvm_unmap_gfn_range() which tears down all of the stage 2 shadow page
> > tables for nested guests via kvm_nested_s2_unmap().
> >
> > To avoid lockup, the kvm->mmu_lock is dropped while doing this and the task
> > rescheduled once for each block of physical address space (32 MiB for 16
> > KiB page size), with the lock being reacquired once the task is scheduled
> > again.
> >
> > This results in a potential race between this L0 tear down and tear down of
> > the guest itself in kvm_flush_shadow_all(), a race which has been observed
> > on real hardware.
> >
> > When this race occurs it causes an invalid kernel warning when the PGT of a
> > nested MMU is cleared by kvm_flush_shadow_all() ->
> > kvm_arch_flush_shadow_all() -> kvm_free_stage2_pgd().
> >
> > Patch 1 fixes this by having stage2_apply_range() no longer return an error
> > when it has experienced a benign race with pgt teardown when it drops the
> > lock.
> >
> > Patch 2 addresses something more serious - bad timing can turn this spurious
> > warning into a NULL pointer dereference.
> >
> > kvm_arch_flush_shadow_all() calls kvm_uninit_stage2_mmu() which calls
> > kvm_free_stage2_pgd() on the canonical kvm->arch.mmu for that guest's S2
> > mappings, making it NULL.
> >
> > This is problematic if it happens before stage2_apply_range() reacquires
> > the kvm->mmu_lock, as it ultimately returns to kvm_nested_s2_unmap() which
> > dereferences kvm->arch.mmu.pgt with the mmu lock held under the incorrect
> > assumption that it means it's valid, resulting in a NULL pointer
> > dereference.
> >
> > Fix that by checking if kvm->arch.mmu.pgt is NULL before dereferencing it
> > in kvm_nested_s2_unmap() and kvm_nested_s2_wp().
>
> With the commit message for patch #1 trimmed to something that fits on
> a couple of standard terminal screens ;-) :

Haha sure will put it on a diet and respin :)

>
> Reviewed-by: Marc Zyngier <maz@kernel.org>

Thanks!

>
> 	M.
>
> --
> Jazz isn't dead. It just smells funny.

--
Cheers, Lorenzo

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

* Re: [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 teardown race
  2026-08-23  5:38   ` Yao Yuan
@ 2026-08-24 13:29     ` Lorenzo Stoakes (ARM)
  0 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-24 13:29 UTC (permalink / raw)
  To: Yao Yuan
  Cc: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
	Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, Christoffer Dall, Wei-Lin Chang, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Sun, Aug 23, 2026 at 01:38:18PM +0800, Yao Yuan wrote:
> On Sat, Aug 22, 2026 at 06:46:53PM +0800, Lorenzo Stoakes (ARM) wrote:
> > A batch of kernel warnings were triggered in the L0 host kernel when using
> > kvmtool to experiment with nested virtualisation.
>
> ...
>
> > Fixes: ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page tables")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  arch/arm64/kvm/mmu.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 74e7e7f7564c..31e049ded093 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -59,19 +59,25 @@ static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
> >   * long will also starve other vCPUs. We have to also make sure that the page
> >   * tables are not freed while we released the lock.
> >   */
> > -static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
> > +static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
> >  			      phys_addr_t end,
> >  			      int (*fn)(struct kvm_pgtable *, u64, u64),
> >  			      bool resched)
> >  {
> >  	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> > +	phys_addr_t addr = start;
> >  	int ret;
> >  	u64 next;
> >
> >  	do {
> >  		struct kvm_pgtable *pgt = mmu->pgt;
> > +		/*
> > +		 * We may be raced on PGT teardown when we release the
> > +		 * kvm->mmu_lock. That's fine as the PGT is legitimately no
> > +		 * longer present.
> > +		 */
> >  		if (!pgt)
> > -			return -EINVAL;
> > +			return resched && addr > start ? 0 : -EINVAL;
>
> Hi,
>
> I can understand the addr checking makes sure only return 0
> when the mmu_lock is dropped at least once and get pgt =
> NULL, may a variable like drop_lock = true when
> cond_resched_rwlock_write(&kvm->mmu_lock) happens can have
> better readability, but depends on you and others' opinion.

Yeah good point, also the condition is resched && next != end, so this was far
too forgiving already :)

So perhaps:

static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
			      phys_addr_t end,
			      int (*fn)(struct kvm_pgtable *, u64, u64),
			      bool resched)
{
	...
	bool lock_dropped = false;
	...
	do {
		...
		if (!pgt)
			return lock_dropped ? 0 : -EINVAL;
		...
		if (resched && next != end) {
			cond_resched_rwlock_write(&kvm->mmu_lock);
			lock_dropped = true;
		}
	} while (...);
	...
}

That keeps it simple and clear and we don't try to infer anything too
complicated from the state just 'the lock was dropped'.

>
> Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>

Thanks :)


>
> >
> >  		next = stage2_range_addr_end(addr, end);
> >  		ret = fn(pgt, addr, next - addr);
> >
> > --
> > 2.55.0

--
Cheers, Lorenzo

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

* Re: [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race
  2026-08-24 13:15   ` Lorenzo Stoakes (ARM)
@ 2026-08-24 15:56     ` Marc Zyngier
  2026-08-24 17:34       ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2026-08-24 15:56 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Christoffer Dall, Wei-Lin Chang, Yao Yuan, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Mon, 24 Aug 2026 14:15:29 +0100,
"Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> 
> On Sun, Aug 23, 2026 at 08:53:31AM +0100, Marc Zyngier wrote:
> > On Sat, 22 Aug 2026 18:46:52 +0100,
> > "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> > >
> > > When GFNs are invalidated in L0 an MMU notifier triggers
> > > kvm_unmap_gfn_range() which tears down all of the stage 2 shadow page
> > > tables for nested guests via kvm_nested_s2_unmap().
> > >
> > > To avoid lockup, the kvm->mmu_lock is dropped while doing this and the task
> > > rescheduled once for each block of physical address space (32 MiB for 16
> > > KiB page size), with the lock being reacquired once the task is scheduled
> > > again.
> > >
> > > This results in a potential race between this L0 tear down and tear down of
> > > the guest itself in kvm_flush_shadow_all(), a race which has been observed
> > > on real hardware.
> > >
> > > When this race occurs it causes an invalid kernel warning when the PGT of a
> > > nested MMU is cleared by kvm_flush_shadow_all() ->
> > > kvm_arch_flush_shadow_all() -> kvm_free_stage2_pgd().
> > >
> > > Patch 1 fixes this by having stage2_apply_range() no longer return an error
> > > when it has experienced a benign race with pgt teardown when it drops the
> > > lock.
> > >
> > > Patch 2 addresses something more serious - bad timing can turn this spurious
> > > warning into a NULL pointer dereference.
> > >
> > > kvm_arch_flush_shadow_all() calls kvm_uninit_stage2_mmu() which calls
> > > kvm_free_stage2_pgd() on the canonical kvm->arch.mmu for that guest's S2
> > > mappings, making it NULL.
> > >
> > > This is problematic if it happens before stage2_apply_range() reacquires
> > > the kvm->mmu_lock, as it ultimately returns to kvm_nested_s2_unmap() which
> > > dereferences kvm->arch.mmu.pgt with the mmu lock held under the incorrect
> > > assumption that it means it's valid, resulting in a NULL pointer
> > > dereference.
> > >
> > > Fix that by checking if kvm->arch.mmu.pgt is NULL before dereferencing it
> > > in kvm_nested_s2_unmap() and kvm_nested_s2_wp().
> >
> > With the commit message for patch #1 trimmed to something that fits on
> > a couple of standard terminal screens ;-) :
> 
> Haha sure will put it on a diet and respin :)

Oliver can probably do so when applying the series.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

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

* Re: [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race
  2026-08-24 15:56     ` Marc Zyngier
@ 2026-08-24 17:34       ` Lorenzo Stoakes (ARM)
  0 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-24 17:34 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Christoffer Dall, Wei-Lin Chang, Yao Yuan, linux-arm-kernel,
	kvmarm, linux-kernel, stable

On Mon, Aug 24, 2026 at 04:56:46PM +0100, Marc Zyngier wrote:
> On Mon, 24 Aug 2026 14:15:29 +0100,
> "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> > > With the commit message for patch #1 trimmed to something that fits on
> > > a couple of standard terminal screens ;-) :
> >
> > Haha sure will put it on a diet and respin :)
>
> Oliver can probably do so when applying the series.

Even better :) though Yao raised a good point on the 1/2 so maybe better for me
to respin with that applied? Or could do as a follow-up.

>
> Thanks,
>
> 	M.
>
> --
> Jazz isn't dead. It just smells funny.

--
Cheers, Lorenzo

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

end of thread, other threads:[~2026-08-24 17:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-22 17:46 [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 teardown race Lorenzo Stoakes (ARM)
2026-08-22 17:46 ` [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 " Lorenzo Stoakes (ARM)
2026-08-23  5:38   ` Yao Yuan
2026-08-24 13:29     ` Lorenzo Stoakes (ARM)
2026-08-22 17:46 ` [PATCH v2 2/2] KVM: arm64: nv: Fix null ptr deref on nested wp/unmap, " Lorenzo Stoakes (ARM)
2026-08-23  7:53 ` [PATCH v2 0/2] KVM: arm64: Fix spurious warn, null ptr deref on S2 " Marc Zyngier
2026-08-24 13:15   ` Lorenzo Stoakes (ARM)
2026-08-24 15:56     ` Marc Zyngier
2026-08-24 17:34       ` Lorenzo Stoakes (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®