Re: [PATCH 27/28] KVM: arm64: Expose FEAT_NV3 to guests

[Marc Zyngier <maz@kernel.org>]

On Thu, 02 Jul 2026 17:39:02 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] A maliciously or incorrectly configured vCPU can trigger a host NULL pointer dereference by requesting an ID register configuration where NV_frac = 0, causing KVM to skip VNCR TLB allocation while still trapping VNCR accesses.
> --
> 
> commit ca23c995d48d1c5c8370918930aa81e2eeb147ab
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Expose FEAT_NV3 to guests
> 
> Further enable FEAT_NV3 by making it visible to NV guests.
> 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index c9bf04944f9cb..64c8fb82fadf6 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -1728,7 +1728,7 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
> >  		 * You get EITHER
> >  		 *
> >  		 * - FEAT_VHE without FEAT_E2H0
> > -		 * - FEAT_NV limited to FEAT_NV2(p1)
> > +		 * - FEAT_NV limited to FEAT_NV2(p1)/NV3
> >  		 * - HCR_EL2.NV1 being RES0
> >  		 *
> >  		 * OR
> > @@ -1740,7 +1740,9 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
> >  		if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features)) {
> >  			val = 0;
> 
> [Severity: Critical]
> This is a pre-existing issue, but could this cause a host NULL pointer
> dereference?
> 
> If userspace configures a vCPU with KVM_ARM_VCPU_HAS_EL2_E2H0, this branch
> forces the entire ID_AA64MMFR4_EL1 register to 0. This includes the NV_frac
> field. Alternatively, userspace could explicitly set ID_AA64MMFR4_EL1.NV_frac
> to 0 via KVM's ID register UAPI.
> 
> When KVM initializes the nested vCPU, it calls kvm_vcpu_allocate_vncr_tlb()
> which checks NV_frac:
> 
> arch/arm64/kvm/nested.c:kvm_vcpu_allocate_vncr_tlb() {
>         if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY))
>                 return 0;
>         ...
> }
> 
> Because NV_frac is 0, the check evaluates to false and the function silently
> returns success without allocating vcpu->arch.vncr_tlb.
> 
> Since vcpu_has_nv() only checks for KVM_ARM_VCPU_HAS_EL2, the vCPU is allowed
> to run. If the guest then accesses a VNCR-trapped register or sets HCR_EL2.NV,

Beeeeeeeep!!!! It looked so good until then...

"VNCR-trapped register" means nothing, as the whole point of VNCR is
to *not* trap, but to write to memory instead. So OK, the page isn't
there, and a write to a register targeting VNCR_EL2 could generate a
fault.

But if ID_AA64MMFR4_EL1.NV_frac==0, then HCR_EL2.NV is RES0:

	NEEDS_FEAT(HCR_EL2_AT		| /* Ignore the original FEAT_NV */
		   HCR_EL2_NV2		|
		   HCR_EL2_NV,
		   feat_nv2),

and feat_nv2 reads:

static bool feat_nv2(struct kvm *kvm)
{
	return ((kvm_has_feat(kvm, ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY) &&
		 kvm_has_feat_enum(kvm, ID_AA64MMFR2_EL1, NV, NI)) ||
		kvm_has_feat(kvm, ID_AA64MMFR2_EL1, NV, NV2));
}

which will return false in the proposed case, making HCR_EL2.NV firmly
RES0. Yes, there's a reason why I spent so much time on this crap.

Since HCR_EL2.NV is RES0, we never ERET to an L2 running at EL2, and
therefore there is never a case where we fault on a guest provided
VNCR page.

> the host exit handler calls kvm_translate_vncr() or kvm_map_l1_vncr(). Both
> functions unconditionally dereference the unallocated vncr_tlb:
> 
> arch/arm64/kvm/nested.c:kvm_translate_vncr() {
>         struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
>         ...
>         vt->valid = false;
>         ...
> }
> 
> Will this lead to a kernel panic when unprivileged userspace configures a
> vCPU this way?

Don't think so.

	M.

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