Re: [PATCH 17/22] KVM: arm64: Set Access flag on table descriptors at stage-1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM incorrectly applies stage-1 Access Flag updates to table descriptors when hardware AF updates are globally disabled, causing spurious permission faults and bypassing nested stage-2 dirty tracking.
--

commit 3e7cbff07bcf9d864e2cd9a33c92f040a6496081
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: Set Access flag on table descriptors at stage-1

This commit implements access flag updates on table descriptors as required
by FEAT_HAFT. Note that unlike leaf descriptors, access flags on table
descriptors never generate a fault.

> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -416,6 +416,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
>  	wi->hd &= (wi->regime == TR_EL2 ?
>  		  FIELD_GET(TCR_EL2_HD, tcr) :
>  		  FIELD_GET(TCR_HD, tcr));
> +	wi->haft  = kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, HAFDBS, HAFT) &&
> +		    FIELD_GET(TCR2_EL1_HAFT, effective_tcr2(vcpu, wi->regime));

[Severity: High]
Should wi->haft initialization be gated by wi->ha?

Per the ARM ARM, TCR2_ELx.HAFT is IGNORED if TCR_ELx.HA is 0. Because
wi->haft is evaluated independently of wi->ha here, the guest could configure
TCR_ELx.HA = 0 but TCR2_ELx.HAFT = 1.

[ ... ]

> @@ -465,7 +467,7 @@ static bool should_set_access_flag(struct s1_walk_info *wi, struct s1_walk_step
>  	if (access->type == WALK_ACCESS_NONARCH)
>  		return false;
>  
> -	return wi->ha;
> +	return kvm_pte_table(ws->desc, ws->level) ? wi->haft : wi->ha;
>  }

If the guest configures TCR_ELx.HA = 0 and TCR2_ELx.HAFT = 1, this will
return true for table descriptors even when hardware AF updates are disabled,
leading to spurious descriptor writes.

This can cause KVM to trigger stage-2 permission faults if the stage-2
mapping is read-only.

Furthermore, if the stage-2 mapping relies on hardware dirty state management
(DBM), KVM's nested stage-2 walk in walk_s1() treats the access as a read
because wi->ha is false:

arch/arm64/kvm/at.c:walk_s1() {
	...
	struct kvm_walk_access s2_access = {
		.type	= WALK_ACCESS_S1PTW,
		.ia	= ws.desc_ipa,
		/*
		 * R_JCXVS, stage-2 dirty state can be updated
		 * for an S1PTW even if the stage-1 descriptor
		 * isn't updated.
		 */
		.write	= wi->ha,
	};
	...
}

This will silently bypass the nested hypervisor's stage-2 dirty tracking
while still modifying the guest memory in host memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623184201.1518871-1-oupton@kernel.org?part=17