Re: [RFC PATCH 09/15] x86/virt/tdx: Add interface to generate a Quote

[Peter Fang <peter.fang@intel.com>]

On Thu, May 28, 2026 at 03:30:45PM -0700, Edgecombe, Rick P wrote:
> > +
> > +	/* TDH.QUOTE.GET expects the input data to fit in a page */
> > +	if (in_data_len > PAGE_SIZE)
> > +		return NULL;
> 
> Do we really need this check? We can't trust the caller to pass the right size?

There is a similar check for this in_data_len on the KVM side in patch
12, but it is for a different reason. The check in KVM is to make sure
it maps valid guest memory pages into the kernel, while here we make
sure it complies with the SEAMCALL API. That said, the KVM check does
make the check here kinda redundant... I can remove this for simplicity.

> 
> > +
> > +	mutex_lock(&tdx_quote_lock);
> > +
> > +	/*
> > +	 * Use the first page of the quote buffer for input data. The buffer
> > +	 * must be at least one page in size. @in_data may not be page-aligned,
> > +	 * but TDH.QUOTE.GET expects page-aligned addresses.
> > +	 */
> > +	memcpy(quote_data.buf, in_data, (size_t)in_data_len);
> > +
> > +	r = tdx_quote_get(td, quote_data.hpa_list[0], (u64)in_data_len,
> > +			  quote_data.hpa_list_pa, quote_data.buf_len, &out_len);
> > +	if (r || !out_len || out_len > quote_data.buf_len)
> 
> 
> How do these various error conditions happen?

"r" is a SEAMCALL error just like any other SEAMCALL. If r == 0
(SUCCESS), there is no documented scenario for when "!out_len" or
"out_len > quote_data.buf_len" would occur. I would assume these would
be TDX module bugs.

The reason I check the last 2 conditions is mainly to protect the
kernel:

  - "!out_len" will cause kvmemdup() to return ZERO_SIZE_PTR
  - "out_len > quote_data.buf_len" will cause out-of-bounds memory
    access in kvmemdup()

> 
> > +		goto out;
> > +
> > +	/*
> > +	 * The quote buffer is a shared resource, so use it only for the
> > +	 * SEAMCALL and copy the data out as soon as possible.
> > +	 */
> > +	quote_dup = kvmemdup(quote_data.buf, out_len, GFP_KERNEL);
> 
> So at init time we allocate a vmalloc for the quote and pre-populate the
> hpa_list. Then we use it every time and copy the contents to a new vmalloc.
> Would it really be that hard to keep the hpa list allocation around, do a
> vmalloc here and update the pfn list. Then do get quote on that and pass back
> the vmalloc we just allocated? Just feels like global reuse way has extra pieces
> in it. Compared to the whole quoting operation, this vmalloc_to_pfn() loop is
> probably not very expensive.

Hm interesting idea. But a Quote buffer could be close to 4MB in the worst
case. Let's say max_quote_size is 3MB, that's 768 vmalloc_to_pfn() calls
each time... That sounds a bit excessive right?

The extra bits mainly come from using kvmemdup() I think. Having to use
kvfree() on it does feel a bit annoying but that was the tradeoff I
made...

>