Re: [PATCH] KVM: arm64: nv: Check RW permissions for insn abort due to S1PTW

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A malicious L1 guest can cause an infinite loop in KVM by configuring a nested stage-2 page table entry as Write-Only, exploiting how KVM handles S1PTW permission faults.
--

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

KVM: arm64: nv: Check RW permissions for insn abort due to S1PTW

This patch modifies permission fault handling for instruction aborts with
S1PTW set. Instead of evaluating execute permissions, it falls through to
the read/write permission check.

Link: https://lore.kernel.org/kvmarm/20260623190607.7106B1F000E9@smtp.kernel.org/

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 94df26de6990c..713fc44f362d9 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
[ ... ]
> @@ -918,7 +918,7 @@ int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu, struct kvm_s2_trans *trans)
>  	if (!kvm_vcpu_trap_is_permission_fault(vcpu))
>  		return 0;
>  
> -	if (kvm_vcpu_trap_is_iabt(vcpu)) {
> +	if (kvm_vcpu_trap_is_iabt(vcpu) && !kvm_vcpu_abt_iss1tw(vcpu)) {
>  		if (vcpu_mode_priv(vcpu))
>  			forward_fault = !kvm_s2_trans_exec_el1(vcpu->kvm, trans);
>  		else

[Severity: High]
This isn't a bug introduced by this patch, but does this change expose
instruction aborts to an infinite loop if the L1 guest maps nested stage-2
memory as Write-Only?

By falling through to the else block in kvm_s2_handle_perm_fault(), S1PTW
instruction aborts are now evaluated for read/write permissions.

However, if the L1 guest mapped the nested stage-2 page table entry as
Write-Only, the hardware will still fault on the missing read permission
during the stage-1 translation table walk.

Looking at kvm_is_write_fault() in arch/arm64/include/asm/kvm_emulate.h:

static inline bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
{
        if (kvm_vcpu_abt_iss1tw(vcpu)) {
                ...
                return kvm_vcpu_trap_is_permission_fault(vcpu);
        }
        ...
}

It unconditionally treats all permission faults on an S1PTW as write faults,
assuming they are Access Flag or Dirty Bit updates.

Back in the else block of kvm_s2_handle_perm_fault():

        bool write_fault = kvm_is_write_fault(vcpu);

        forward_fault = ((write_fault && !trans->writable) ||
                         (!write_fault && !trans->readable));

Because write_fault is true, it only evaluates !trans->writable. Since the
mapping is Write-Only, this check passes and KVM doesn't forward the fault.
Instead, KVM resumes the guest, which will immediately fault again on the
read, causing an infinite loop.

Is there a way to ensure we also check trans->readable when the walk is
failing due to missing read permissions?

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