Re: [PATCH v4 4/9] KVM: TDX: Don't assume exit_reason[31:16] is all-0 in tdx_to_vmx_exit_reason()

[email protected]
Newsgroups org.kernel.vger.kvm
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Strict equality checks on the raw 64-bit `vp_enter_ret` fail when Bus Lock VM exit sets bit 26, bypassing the intended `KVM_BUG_ON()` and fastpath logic.
--

--- Patch [4]: [PATCH v4 4/9] KVM: TDX: Don't assume exit_reason[31:16] is all-0 in tdx_to_vmx_exit_reason() ---
commit 48df7e5971adbfae6378e9f4d893dc4fe8eb125f
Author: Xiaoyao Li <[email protected]>

KVM: TDX: Don't assume exit_reason[31:16] is all-0 in tdx_to_vmx_exit_reason()

This commit changes tdx_to_vmx_exit_reason() to handle the upper 16 bits
of the exit reason instead of assuming they are all zero. It changes the
return type to union vmx_exit_reason to make manipulating the basic field
more convenient while preserving the upper bits for when Bus Lock VM exit
is enabled.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 4e275cb6927ad..9870922839552 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
[ ... ]
> @@ -949,22 +949,26 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>  	 * but this is NOT a failed VM-Enter.
>  	 */
>  	if (!tdx_is_exit_reason_valid(tdx->vp_enter_ret))
> -		return TDX_INVALID_EXIT_REASON;
> +		return (union vmx_exit_reason) {
> +			.basic = TDX_INVALID_EXIT_REASON,
> +		};
>  
> -	exit_reason = tdx->vp_enter_ret;
> +	exit_reason.full = (u32)tdx->vp_enter_ret;
>  
> -	switch (exit_reason) {
> +	switch (exit_reason.basic) {
>  	case EXIT_REASON_TDCALL:
>  		if (tdvmcall_exit_type(vcpu))
> -			return EXIT_REASON_VMCALL;
> -
> -		return tdcall_to_vmx_exit_reason(vcpu);
> +			exit_reason.basic = EXIT_REASON_VMCALL;
> +		else
> +			exit_reason.basic = tdcall_to_vmx_exit_reason(vcpu);
> +		break;
>  	case EXIT_REASON_EPT_MISCONFIG:
>  		/*
>  		 * Defer KVM_BUG_ON() until tdx_handle_exit() because this is in
>  		 * non-instrumentable code with interrupts disabled.
>  		 */
> -		return TDX_INVALID_EXIT_REASON;
> +		exit_reason.basic = TDX_INVALID_EXIT_REASON;
> +		break;

[Severity: Medium]
This is a pre-existing issue, but since we are preparing for Bus Lock VM exits
which can set bit 26 in the exit reason, will the deferred checks in other
functions still work correctly?

Looking at tdx_handle_exit() in arch/x86/kvm/vmx/tdx.c, it checks the raw
vp_enter_ret directly against EXIT_REASON_EPT_MISCONFIG:

	if (unlikely(vp_enter_ret == EXIT_REASON_EPT_MISCONFIG)) {
		KVM_BUG_ON(1, vcpu->kvm);
		return -EIO;
	}

Similarly, tdx_vcpu_run() also performs a strict equality check:

	if (unlikely(tdx->vp_enter_ret == EXIT_REASON_EPT_MISCONFIG))
		return EXIT_FASTPATH_NONE;

If a bus lock VM exit occurs concurrently with an EPT misconfig, setting bit
26 in vp_enter_ret, won't these strict equality checks fail and bypass the
intended bug-handling paths? Should these checks be updated to mask the upper
bits or use the basic exit reason before enabling Bus Lock VM exit?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.