Re: [PATCH v3 3/4] KVM: TDX: Don't assume exit_reason[31:16] as all-0 in tdx_to_vmx_exit_reason()

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 8/12/2026 4:23 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] `tdx_get_exit_info()` incorrectly compares the full 32-bit exit reason against the 16-bit `TDX_INVALID_EXIT_REASON`, exposing meaningless/inconsistent exit qualification and interrupt info.
> 
> Pre-existing issues:
> - [High] Strict equality checks against `vp_enter_ret` in `tdx_handle_exit()` and `tdx_vcpu_run()` fail to mask the basic exit reason, bypassing critical bug checks when upper exit reason bits are set.
> --
> 
> --- Patch [3]: [PATCH v3 3/4] KVM: TDX: Don't assume exit_reason[31:16] as all-0 in tdx_to_vmx_exit_reason() ---
> commit 129e5cab6b9416a1d4e5f1f8c42f24dc1a44940c
> Author: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> KVM: TDX: Don't assume exit_reason[31:16] as all-0 in tdx_to_vmx_exit_reason()
>      
> When handling the real Exit Reason, don't assume the upper 16 bits as
> all-0 in tdx_to_vmx_exit_reason(), in preparation for enabling Bus Lock
> VM exit. Also preserve the bit 31:16 when changing the (basic) Exit
> Reason, to not lose the information in bit[31:16].
> 
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index df23db9430f06..598b85d772e37 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
> [ ... ]
>> @@ -942,23 +942,27 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>>   		 * module never attempted to run the vCPU, i.e. the Exit
>>   		 * Reason is undefined, but this is NOT a failed VM-Enter
>>   		 */
>> -		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]
> When the basic exit reason is set to TDX_INVALID_EXIT_REASON here, but upper
> bits like Bus Lock are preserved in exit_reason.full, does this break
> tdx_get_exit_info()?
> 
> In tdx_get_exit_info(), it evaluates against the full 32-bit exit reason:
> 
>      *reason = tdx->vt.exit_reason.full;
>      if (*reason != TDX_INVALID_EXIT_REASON) {
>          *info1 = vmx_get_exit_qual(vcpu);
>          ...
> 
> Since TDX_INVALID_EXIT_REASON is the 16-bit 0xFFFF, this check will pass if
> any upper bits are set, even if the basic reason was marked invalid,
> incorrectly exposing meaningless hardware state.

Oh, I should have called it out it's intentional to check the full 32 
bits Exit Reason instead of checking the lower 16 bits .basic field as 
what it did in the v2.

The purpose here is to return zero value for info1/info2/intr_info when 
there is no real Exit Reason, i.e., no real VM-Enter has happened. "Full 
32 bits Exit Reason equals TDX_INVALID_EXIT_REASON" indicates the case 
of no real Exit Reason.

There is one exception though: when real EPT_MISCONFIG happens without 
bus_lock_detected being 1. In this case, it's a false positive and KVM 
should go to the else path to read the info1/info2/intr_info. As noted 
in patch 2 under "---", this is an pre-existing issue and it will be 
address in a future series.

For the case that Sashiko cares, the only possible one is that real 
EPT_MISCONFIG happens when bus_lock_detected. In this case, passing the 
check of (*reason != TDX_INVALID_EXIT_REASON) to read the value of 
info1/info2/intr_info happens to be what we expect. (However, the exit 
reason is 0xFFFF instead of EPT_MISCONFIG). Anyway, it won't expose 
meaningless hardware state.

So, considering that the false-positive on EPT_MISCONFIG cannot be fully 
addressed with either checking the full 32 bits or checking the 16 bits 
basic field, and checking the full 32 bits can ensure identifying the no 
real Exit Reason cases. I just leave it as-is instead of changing it to 
check the basic field.

> [Severity: High]
> This isn't a bug introduced by this patch, but by highlighting that
> vp_enter_ret can contain upper exit reason bits (like bit 26), does this
> expose a flaw in the strict equality checks for EPT misconfigurations?
> 
> In tdx_vcpu_run(), the check is strictly against the 64-bit value:
> 
>      if (unlikely(tdx->vp_enter_ret == EXIT_REASON_EPT_MISCONFIG))
>          return EXIT_FASTPATH_NONE;
> 
> Similarly, in tdx_handle_exit(), the bug check is also a strict comparison:
> 
>      if (unlikely(vp_enter_ret == EXIT_REASON_EPT_MISCONFIG)) {
>          KVM_BUG_ON(1, vcpu->kvm);
>          return -EIO;
>      }
> 
> If a hardware EPT misconfig occurs while upper bits are set in the raw
> vp_enter_ret, won't these checks fail to identify the misconfig, bypassing
> the KVM_BUG_ON() entirely?

If it happens, it bypasses the KVM_BUG_ON(). But it will finally exit to 
userspace due to invalid Exit Reason, 0xFFFF.

Indeed, it may cause behavior change. But considering the EPT_MISCONFIG 
is not supposed to happen with current KVM's code, and we want to 
backport the series to stable kernels. We don't want to handle the 
theoretical issue that cannot happen in practice with this series, to 
make the series simple. We can harden the EPT_MISCONFIG code with 
another series separately.

>>   	default:
>>   		break;
>>   	}
>>
>>   	return exit_reason;
>>   }
>