Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM incorrectly returns a hypercall error (TDVMCALL_STATUS_INVALID_OPERAND) when in-kernel quote generation gracefully fails.
- [High] KVM fails to write the required buffer size back to the guest when the provided GetQuote buffer is too small.
--

commit 482cde6e1c4a3f88b17ca20d55ddbc439d933981
Author: Peter Fang <peter.fang@intel.com>

KVM: TDX: Add in-kernel Quote generation

Provide an in-kernel path for Quote generation when handling
TDG.VP.VMCALL<GetQuote>, without requiring an exit to userspace.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 9f7c39e0d4b59..20558b0185b63 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c

[ ... ]

> +static u64 get_quote_kernel(struct kvm_vcpu *vcpu, struct tdx_quote_req *req,
> +			    gpa_t req_gpa, size_t total_len)
> +{

[ ... ]

> +	/* Caller owns the requested quote */
> +	void *quote_data __free(kvfree) =
> +		tdx_quote_generate(td, req->data, req->in_len, &req->out_len);
> +
> +	if (!quote_data)
> +		return TDX_QUOTE_STATUS_UNAVAILABLE;
> +
> +	if ((size_t)req->out_len + TDX_QUOTE_REQ_HDR_SIZE > total_len)
> +		return TDX_QUOTE_STATUS_ERROR;

[Severity: High]
If the guest-provided buffer is too small, does this early return skip
writing the required size back to the guest?

When tdx_quote_generate() fails due to an insufficient buffer, it populates
req->out_len with the required size. By returning TDX_QUOTE_STATUS_ERROR
immediately here, it seems we bypass the logic that writes req->out_len back
to the guest.

Will this break a guest's ability to dynamically probe the required quote size
and retry if its initial allocation is too small?

> +
> +	if (!write_quote_to_guest(vcpu, quote_data, req->out_len, req_gpa))
> +		return TDX_QUOTE_STATUS_ERROR;
> +
> +	return TDX_QUOTE_STATUS_SUCCESS;
> +}

[ ... ]

> +static int tdx_get_quote_kernel(struct kvm_vcpu *vcpu, u64 gpa, u64 size)
> +{
> +	void *first_page = NULL;
> +	u64 err, qerr;
> +
> +	err = tdx_get_quote_check_args(vcpu, gpa, size);
> +	if (err != TDVMCALL_STATUS_SUCCESS)
> +		goto out;
> +
> +	err = TDVMCALL_STATUS_INVALID_OPERAND;

[ ... ]

> +	qerr = get_quote_kernel(vcpu, first_page, (gpa_t)gpa, size);
> +
> +	if (write_quote_status_to_guest(vcpu, qerr, (gpa_t)gpa) &&
> +	    qerr == TDX_QUOTE_STATUS_SUCCESS)
> +		err = TDVMCALL_STATUS_SUCCESS;

[Severity: High]
Should the hypercall return SUCCESS even if the quote generation itself
failed gracefully?

If get_quote_kernel() encounters a quote service error (like
TDX_QUOTE_STATUS_UNAVAILABLE), this condition prevents err from being updated
to TDVMCALL_STATUS_SUCCESS. As a result, the hypercall returns the
initialized value TDVMCALL_STATUS_INVALID_OPERAND in R10.

Since the hypercall arguments were structurally valid and we successfully
wrote the error status into the shared memory buffer, shouldn't the hypercall
return SUCCESS to let the guest parse the actual quote status?

> +
> +out:
> +	kfree(first_page);
> +	tdvmcall_set_return_code(vcpu, err);
> +
> +	return 1;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618081355.3253581-1-yilun.xu@linux.intel.com?part=16