Re: [PATCH v3 2/5] KVM: arm64: nv: Inject SEA if kvm_translate_vncr() can't resolve PFN

[sashiko-bot@kernel.org]

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

New issues:
- [High] Unhandled guest_memfd errors ignore prepared memory fault exits and inject malformed guest exceptions.

Pre-existing issues:
- [High] Host-side PFN resolution errors (e.g., SIGPENDING, HWPOISON) are silently converted into guest Synchronous External Aborts (SEA).
--

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

KVM: arm64: nv: Inject SEA if kvm_translate_vncr() can't resolve PFN

This commit updates kvm_translate_vncr() to prepare an external abort
when failing to resolve a PFN instead of returning -EFAULT and relying
on kvm_handle_vncr_abort() to BUG_ON() or correctly guess the failure.
It drops the BUG_ON() in the abort handler to safely inject the SEA.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 903ccabca78c5..53dea9c3f14f8 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1395,15 +1395,19 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  
>  	gfn = vt->wr.pa >> PAGE_SHIFT;
>  	memslot = gfn_to_memslot(vcpu->kvm, gfn);
> -	if (!memslot)
> +	if (!memslot) {
> +		fail_s1_walk(&vt->wr, ESR_ELx_FSC_EXTABT, false);
>  		return -EFAULT;
> +	}
>  
>  	*is_gmem = kvm_slot_has_gmem(memslot);
>  	if (!*is_gmem) {
>  		pfn = __kvm_faultin_pfn(memslot, gfn, write_fault ? FOLL_WRITE : 0,
>  					&writable, &page);
> -		if (is_error_noslot_pfn(pfn))
> +		if (is_error_noslot_pfn(pfn)) {
> +			fail_s1_walk(&vt->wr, ESR_ELx_FSC_EXTABT, false);
>  			return -EFAULT;
> +		}

[Severity: High]
This is a pre-existing issue, but does this improperly convert host PFN
resolution errors into guest aborts?

If __kvm_faultin_pfn() encounters a pending host signal or poisoned memory,
it can return KVM_PFN_ERR_SIGPENDING or KVM_PFN_ERR_HWPOISON. These are
caught by is_error_noslot_pfn().

By calling fail_s1_walk() and returning -EFAULT here, kvm_handle_vncr_abort()
will inject a Synchronous External Abort (SEA) into the guest instead of
returning to userspace. Will this cause a simple host signal to randomly
crash the VM and break host HWPOISON recovery?

[ ... ]

> @@ -1530,8 +1534,6 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
>  			 * Translation failed, inject the corresponding
>  			 * exception back to EL2.
>  			 */
> -			BUG_ON(!vt->wr.failed);
> -

[Severity: High]
Does removing this BUG_ON() unmask an issue when kvm_gmem_get_pfn() fails
with -EINVAL, -ENOENT, or -EACCES?

In kvm_translate_vncr(), if kvm_gmem_get_pfn() fails, it prepares a memory
fault exit for userspace and returns the error code without calling
fail_s1_walk().

Because kvm_handle_vncr_abort() lacks an 'if (is_gmem) return 0;' check for
-EINVAL, -ENOENT, and -EACCES, it will fall through to the exception injection
path below. Since vt->wr.fst is uninitialized (0), won't KVM ignore the
prepared memory fault exit and inject a malformed exception (FSC=0) into the
guest?

>  			esr &= ~ESR_ELx_FSC;
>  			esr |= FIELD_PREP(ESR_ELx_FSC, vt->wr.fst);
>

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