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

[sashiko-bot@kernel.org]

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,
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?

>  		} else {
> -			if (cpus_have_final_cap(ARM64_HAS_NV2P1))
> +			if (cpus_have_final_cap(ARM64_HAS_NV3))
> +				val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR4_EL1, NV_frac, NV3);
> +			else if (cpus_have_final_cap(ARM64_HAS_NV2P1))
>  				val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR4_EL1, NV_frac, NV2P1);
>  			else
>  				val = SYS_FIELD_PREP_ENUM(ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702160248.1377250-1-maz@kernel.org?part=27