Re: [PATCH v3 16/28] KVM: arm64: Split NV-specific exit fixups from the non-NV handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `inline` from `fixup_guest_exit()` and breaking out `fixup_nv_guest_exit()` exposes them to kprobes, which will cause a fatal host panic if probed.
--

--- Patch [16]: [PATCH v3 16/28] KVM: arm64: Split NV-specific exit fixups from the non-NV handling ---
commit 54c98942c16b88b656ffdb68540aac18e6fe4d3f
Author: Marc Zyngier <maz@kernel.org>
KVM: arm64: Split NV-specific exit fixups from the non-NV handling

This commit moves the nested virtualization (NV) handling of early fixups 
into its own helper function. This simplifies parsing and prepares the code
for further changes.

> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index a8da40568a048..53af19212895d 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -539,18 +539,15 @@ static const exit_handler_fn hyp_exit_handlers[] = {
>  	[0x3F]				= kvm_hyp_handle_impdef,
>  };
>  
> -static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> +static void fixup_nv_guest_exit(struct kvm_vcpu *vcpu)

[Severity: High]
Does extracting this function without a NOKPROBE_SYMBOL() annotation expose
it to kprobes?

Since this executes on the VM exit path while the KVM hypervisor vectors are
still loaded, placing a kprobe here would trigger a BRK instruction that traps
to __kvm_hyp_vector. This cannot handle kprobes and would unconditionally
call hyp_panic(), crashing the host.

Should we add NOKPROBE_SYMBOL(fixup_nv_guest_exit) to prevent this?

>  {
> -	synchronize_vcpu_pstate(vcpu);
> -
>  	/*
>  	 * If we were in HYP context on entry, adjust the PSTATE view
>  	 * so that the usual helpers work correctly. This enforces our
>  	 * invariant that the guest's HYP context status is preserved
>  	 * across a run.
>  	 */
> -	if (vcpu_has_nv(vcpu) &&
> -	    unlikely(host_data_test_flag(VCPU_IN_HYP_CONTEXT))) {
> +	if (unlikely(host_data_test_flag(VCPU_IN_HYP_CONTEXT))) {

[ ... ]

> @@ -567,8 +564,15 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	}
>  
>  	/* Apply extreme paranoia! */
> -	BUG_ON(vcpu_has_nv(vcpu) &&
> -	       !!host_data_test_flag(VCPU_IN_HYP_CONTEXT) != is_hyp_ctxt(vcpu));
> +	BUG_ON(!!host_data_test_flag(VCPU_IN_HYP_CONTEXT) != is_hyp_ctxt(vcpu));
> +}
> +
> +static bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)

[Severity: High]
By removing the inline keyword here, doesn't this also expose
fixup_guest_exit() to kprobes if the compiler chooses not to inline it?

Should this function also receive a NOKPROBE_SYMBOL() annotation to avoid the
same host panic hazard?

> +{
> +	synchronize_vcpu_pstate(vcpu);
> +
> +	if (vcpu_has_nv(vcpu))
> +		fixup_nv_guest_exit(vcpu);
>  
>  	return __fixup_guest_exit(vcpu, exit_code, hyp_exit_handlers);
>  }

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