Re: [PATCH v2 2/3] KVM: TDX: Fix the exit reason handling

[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:
- [Medium] The condition `tdx->vt.exit_reason.basic != -1` in `tdx_get_exit_info()` is unconditionally true due to C integer promotion rules, defeating the check for synthesized invalid exit reasons and leaking uninitialized or host state to the `kvm_exit` tracepoint.
--

--- Patch [2]: [PATCH v2 2/3] KVM: TDX: Fix the exit reason handling ---
commit d21d0519253cbb288be7d1030cb7a0bb7547067a
Author: Xiaoyao Li <[email protected]>

KVM: TDX: Fix the exit reason handling

This patch modifies how KVM handles TDX exit reasons by isolating
the lower 16 bits into the basic field and keeping the upper 16 bits
unchanged. It synthesizes an invalid exit reason (-1) for specific
TDX SEAMCALL statuses and updates tdx_get_exit_info() to check this field.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 66e9e6ee62dae..2c8844acbd43e 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c

[ ... ]

> @@ -2147,7 +2156,7 @@ void tdx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason,
>  	struct vcpu_tdx *tdx = to_tdx(vcpu);
>  
>  	*reason = tdx->vt.exit_reason.full;
> -	if (*reason != -1u) {
> +	if (tdx->vt.exit_reason.basic != -1) {

[Severity: Medium]
Will this check always evaluate to true due to C integer promotion rules?

The basic field in union vmx_exit_reason is a 16-bit unsigned bitfield. When
comparing it to -1, the unsigned 16-bit value is promoted to a signed 32-bit
integer. If the value was set to -1 (65535), the comparison evaluates as
65535 != -1, which is always true.

Could this cause uninitialized or host state (such as tdvpr_pa passed via rcx
to the TDX module) to leak to the kvm_exit tracepoint instead of falling back
and safely zeroing the exit data?

>  		*info1 = vmx_get_exit_qual(vcpu);
>  		*info2 = tdx->ext_exit_qualification;
>  		*intr_info = vmx_get_intr_info(vcpu);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
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.