public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
@ 2026-02-02 13:04 Fuad Tabba
  2026-02-02 14:45 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Fuad Tabba @ 2026-02-02 13:04 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel
  Cc: maz, oliver.upton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will, qperret, tabba

KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range

Protected mode uses `pkvm_mappings` of the union inside `struct kvm_pgtable`.
This aliases `ia_bits`, which is used in non-protected mode.

Attempting to use `pgt->ia_bits` in kvm_nested_s2_unmap() and
kvm_nested_s2_wp() results in reading mapping pointers or state as a
shift amount. This triggers a UBSAN shift-out-of-bounds error:

    UBSAN: shift-out-of-bounds in arch/arm64/kvm/nested.c:1127:34
    shift exponent 174565952 is too large for 64-bit type 'unsigned long'
    Call trace:
     __ubsan_handle_shift_out_of_bounds+0x28c/0x2c0
     kvm_nested_s2_unmap+0x228/0x248
     kvm_arch_flush_shadow_memslot+0x98/0xc0
     kvm_set_memslot+0x248/0xce0

Fix this by using kvm_phys_size() to determine the IPA size. This helper
is independent of the software page table representation and works
correctly for both protected and non-protected modes, as it derives the
size directly from VTCR_EL2.

Fixes: 7270cc9157f47 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU notifiers")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
Based on Linux 6.19-rc8
---
 arch/arm64/kvm/nested.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index cdeeb8f09e72..eb9666ba15b4 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1108,7 +1108,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(kvm, 0, kvm_phys_size(&kvm->arch.mmu));
 }
 
 void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
@@ -1124,7 +1124,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(kvm, 0, kvm_phys_size(&kvm->arch.mmu));
 }
 
 void kvm_nested_s2_flush(struct kvm *kvm)

base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
-- 
2.53.0.rc1.225.gd81095ad13-goog



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

* Re: [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
  2026-02-02 13:04 [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range Fuad Tabba
@ 2026-02-02 14:45 ` Marc Zyngier
  2026-02-02 14:54   ` Fuad Tabba
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2026-02-02 14:45 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: kvm, kvmarm, linux-arm-kernel, oliver.upton, joey.gouly,
	suzuki.poulose, yuzenghui, catalin.marinas, will, qperret

On Mon, 02 Feb 2026 13:04:24 +0000,
Fuad Tabba <tabba@google.com> wrote:
> 
> KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
> 
> Protected mode uses `pkvm_mappings` of the union inside `struct kvm_pgtable`.
> This aliases `ia_bits`, which is used in non-protected mode.
> 
> Attempting to use `pgt->ia_bits` in kvm_nested_s2_unmap() and
> kvm_nested_s2_wp() results in reading mapping pointers or state as a
> shift amount. This triggers a UBSAN shift-out-of-bounds error:
> 
>     UBSAN: shift-out-of-bounds in arch/arm64/kvm/nested.c:1127:34
>     shift exponent 174565952 is too large for 64-bit type 'unsigned long'
>     Call trace:
>      __ubsan_handle_shift_out_of_bounds+0x28c/0x2c0
>      kvm_nested_s2_unmap+0x228/0x248
>      kvm_arch_flush_shadow_memslot+0x98/0xc0
>      kvm_set_memslot+0x248/0xce0
> 
> Fix this by using kvm_phys_size() to determine the IPA size. This helper
> is independent of the software page table representation and works
> correctly for both protected and non-protected modes, as it derives the
> size directly from VTCR_EL2.

I'm a bit confused by the explanation. We have plenty of code that
uses pgt->ia_bits outside of the NV code. And yet that code is not
affected by this?

I'm asking because NV is clearly a case where the pkvm_mappings
aliasing is unambiguously *not* happening.

Isn't the real issue that we are entering the NV handling code for any
S2 manipulation irrespective of NV support? Would something like below
help instead?

Thanks,

	M.

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index cdeeb8f09e722..d03e9b71bf6cd 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1101,6 +1101,9 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
+	if (!kvm->arch.nested_mmus_size)
+		return;
+
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
 		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
 
@@ -1117,6 +1120,9 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
+	if (!kvm->arch.nested_mmus_size)
+		return;
+
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
 		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
 
@@ -1133,6 +1139,9 @@ void kvm_nested_s2_flush(struct kvm *kvm)
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
+	if (!kvm->arch.nested_mmus_size)
+		return;
+
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
 		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
 
@@ -1145,6 +1154,9 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
 {
 	int i;
 
+	if (!kvm->arch.nested_mmus_size)
+		return;
+
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
 		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
 

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
  2026-02-02 14:45 ` Marc Zyngier
@ 2026-02-02 14:54   ` Fuad Tabba
  2026-02-02 15:04     ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Fuad Tabba @ 2026-02-02 14:54 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm, kvmarm, linux-arm-kernel, oliver.upton, joey.gouly,
	suzuki.poulose, yuzenghui, catalin.marinas, will, qperret

Hi Marc,

On Mon, 2 Feb 2026 at 14:45, Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 02 Feb 2026 13:04:24 +0000,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
> >
> > Protected mode uses `pkvm_mappings` of the union inside `struct kvm_pgtable`.
> > This aliases `ia_bits`, which is used in non-protected mode.
> >
> > Attempting to use `pgt->ia_bits` in kvm_nested_s2_unmap() and
> > kvm_nested_s2_wp() results in reading mapping pointers or state as a
> > shift amount. This triggers a UBSAN shift-out-of-bounds error:
> >
> >     UBSAN: shift-out-of-bounds in arch/arm64/kvm/nested.c:1127:34
> >     shift exponent 174565952 is too large for 64-bit type 'unsigned long'
> >     Call trace:
> >      __ubsan_handle_shift_out_of_bounds+0x28c/0x2c0
> >      kvm_nested_s2_unmap+0x228/0x248
> >      kvm_arch_flush_shadow_memslot+0x98/0xc0
> >      kvm_set_memslot+0x248/0xce0
> >
> > Fix this by using kvm_phys_size() to determine the IPA size. This helper
> > is independent of the software page table representation and works
> > correctly for both protected and non-protected modes, as it derives the
> > size directly from VTCR_EL2.
>
> I'm a bit confused by the explanation. We have plenty of code that
> uses pgt->ia_bits outside of the NV code. And yet that code is not
> affected by this?
>
> I'm asking because NV is clearly a case where the pkvm_mappings
> aliasing is unambiguously *not* happening.
>
> Isn't the real issue that we are entering the NV handling code for any
> S2 manipulation irrespective of NV support? Would something like below
> help instead?

That would definitely work (just tested it). I just assumed that the
code is there in case in the future we want to support nv + pkvm....
Although, I chuckled a bit as I was writing those words :)

I was going to ask if you'd like me to respin, but this is a
completely different patch. Would you like me to write it up and send
it (my contribution would be the commit msg)?

Cheers,
/fuad


> Thanks,
>
>         M.
>
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index cdeeb8f09e722..d03e9b71bf6cd 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1101,6 +1101,9 @@ void kvm_nested_s2_wp(struct kvm *kvm)
>
>         lockdep_assert_held_write(&kvm->mmu_lock);
>
> +       if (!kvm->arch.nested_mmus_size)
> +               return;
> +
>         for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
>                 struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
>
> @@ -1117,6 +1120,9 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
>
>         lockdep_assert_held_write(&kvm->mmu_lock);
>
> +       if (!kvm->arch.nested_mmus_size)
> +               return;
> +
>         for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
>                 struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
>
> @@ -1133,6 +1139,9 @@ void kvm_nested_s2_flush(struct kvm *kvm)
>
>         lockdep_assert_held_write(&kvm->mmu_lock);
>
> +       if (!kvm->arch.nested_mmus_size)
> +               return;
> +
>         for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
>                 struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
>
> @@ -1145,6 +1154,9 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
>  {
>         int i;
>
> +       if (!kvm->arch.nested_mmus_size)
> +               return;
> +
>         for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
>                 struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
>
>
> --
> Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
  2026-02-02 14:54   ` Fuad Tabba
@ 2026-02-02 15:04     ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2026-02-02 15:04 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: kvm, kvmarm, linux-arm-kernel, oliver.upton, joey.gouly,
	suzuki.poulose, yuzenghui, catalin.marinas, will, qperret

On Mon, 02 Feb 2026 14:54:55 +0000,
Fuad Tabba <tabba@google.com> wrote:
> 
> Hi Marc,
> 
> On Mon, 2 Feb 2026 at 14:45, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Mon, 02 Feb 2026 13:04:24 +0000,
> > Fuad Tabba <tabba@google.com> wrote:
> > >
> > > KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range
> > >
> > > Protected mode uses `pkvm_mappings` of the union inside `struct kvm_pgtable`.
> > > This aliases `ia_bits`, which is used in non-protected mode.
> > >
> > > Attempting to use `pgt->ia_bits` in kvm_nested_s2_unmap() and
> > > kvm_nested_s2_wp() results in reading mapping pointers or state as a
> > > shift amount. This triggers a UBSAN shift-out-of-bounds error:
> > >
> > >     UBSAN: shift-out-of-bounds in arch/arm64/kvm/nested.c:1127:34
> > >     shift exponent 174565952 is too large for 64-bit type 'unsigned long'
> > >     Call trace:
> > >      __ubsan_handle_shift_out_of_bounds+0x28c/0x2c0
> > >      kvm_nested_s2_unmap+0x228/0x248
> > >      kvm_arch_flush_shadow_memslot+0x98/0xc0
> > >      kvm_set_memslot+0x248/0xce0
> > >
> > > Fix this by using kvm_phys_size() to determine the IPA size. This helper
> > > is independent of the software page table representation and works
> > > correctly for both protected and non-protected modes, as it derives the
> > > size directly from VTCR_EL2.
> >
> > I'm a bit confused by the explanation. We have plenty of code that
> > uses pgt->ia_bits outside of the NV code. And yet that code is not
> > affected by this?
> >
> > I'm asking because NV is clearly a case where the pkvm_mappings
> > aliasing is unambiguously *not* happening.
> >
> > Isn't the real issue that we are entering the NV handling code for any
> > S2 manipulation irrespective of NV support? Would something like below
> > help instead?
> 
> That would definitely work (just tested it). I just assumed that the
> code is there in case in the future we want to support nv + pkvm....
> Although, I chuckled a bit as I was writing those words :)

Don't laugh, I seriously considered what it would take to teach NV to
the RMM, just as a way to get rid of the ridiculous notion of planes
(which is exactly like NV, only done in a way that is even worse than
the architected version, so even more costly for no good reason).

> I was going to ask if you'd like me to respin, but this is a
> completely different patch. Would you like me to write it up and
> send it (my contribution would be the commit msg)?

Yes please, it's a lot less effort for me! :p

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.


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

end of thread, other threads:[~2026-02-02 15:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-02 13:04 [PATCH v1] KVM: arm64: nv: Use kvm_phys_size() for VNCR invalidation range Fuad Tabba
2026-02-02 14:45 ` Marc Zyngier
2026-02-02 14:54   ` Fuad Tabba
2026-02-02 15:04     ` Marc Zyngier

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