Re: [PATCH v17 07/14] KVM: arm64: Forward GCS exceptions to nested guests

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] GCS data check exceptions from the L1 hypervisor context incorrectly trigger a host-side WARN_ON_ONCE and an UNDEFINED exception injection instead of being forwarded.
--

--- Patch [7]: [PATCH v17 07/14] KVM: arm64: Forward GCS exceptions to nested guests ---
commit c532b4ba16abdf3ea91375bafb69e02b935e5151
Author: Mark Brown <[email protected]>

KVM: arm64: Forward GCS exceptions to nested guests

This commit adds handling to forward GCS exceptions to the hypervisor
when a nested guest has HFGITR_EL2.nGCSSTR_EL1 clear or for a GCS data
check exception in an L2 guest.

> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 54aedf93c78b6..9db3508917937 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
[ ... ]
> +/*
> + * We might get GCS exceptions that need to be forwarded to the
> + * hypervisor when a nested guest has HFGITR_EL2.nGCSSTR_EL1 clear, or
> + * for a GCS data check exception for a L2 guest.
> + */
>  static int kvm_handle_gcs(struct kvm_vcpu *vcpu)
>  {
> -	/* We don't expect GCS, so treat it with contempt */
> -	if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> -		WARN_ON_ONCE(1);
> +	if (!kvm_has_gcs(vcpu->kvm)) {
> +		kvm_inject_undefined(vcpu);
> +		return 1;
> +	}
>  
> +	if (is_nested_ctxt(vcpu)) {

[Severity: High]
Does is_nested_ctxt() incorrectly filter out the L1 hypervisor context?

If a VHE L1 hypervisor (or its L1 host userspace) executes a GCS instruction
that generates a GCS data check exception, the hardware targets EL1. Due to
HCR_EL2.NV = 1, this is redirected to L0 KVM.

Since is_nested_ctxt(vcpu) returns false for L1 hypervisor contexts
(because is_hyp_ctxt(vcpu) is true), this check will fail and fall through
to injecting an UNDEFINED exception.

Should this use vcpu_has_nv(vcpu) instead to ensure exceptions from both L1
and L2 contexts are properly forwarded to the virtual EL2?

> +		kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu));
> +		return 1;
> +	}
> +
> +	WARN_ON_ONCE(1);

[Severity: High]
Is it possible for an unprivileged L1 userspace process using GCS to trigger
this WARN_ON_ONCE?

Since the is_nested_ctxt() check above falls through for L1 contexts, a
legitimate GCS fault in L1 (such as mismatching a return address on the shadow
stack) could reach this warning. This would allow an L1 guest to trigger a
host-side kernel warning (kernel log DoS) and crash the L1 VM.

>  	kvm_inject_undefined(vcpu);
>  	return 1;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7