Re: [PATCH v6 6/6] virt: tdx-guest: Make the Quote buffer size dynamic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] virt: tdx-guest: TOCTOU on get_quote_buf_size() causes out-of-bounds write
--

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

virt: tdx-guest: Make the Quote buffer size dynamic

This commit updates the TDX guest driver to dynamically size the Quote buffer
based on the TDX module's reported limits, supporting larger certificates like
PQC. It falls back to a default 128KB size if the module does not report a
limit.

> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 0cf078f09a73b..11d741da3b35f 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c

[ ... ]

> @@ -209,7 +209,20 @@ static long tdx_get_report0(struct tdx_report_req __user *req)
>  /* Size of the header metadata plus the largest possible raw Quote. */
>  static size_t get_quote_buf_size(void)
>  {
> -	return TDX_DEFAULT_QUOTE_SIZE;
> +	size_t buf_size;
> +	u64 max_size;
> +
> +	/* Start with the default buffer size, which includes the header */
> +	buf_size = TDX_DEFAULT_QUOTE_SIZE;
> +
> +	/*
> +	 * Override the default when the TDX module reports a size. Add room
> +	 * for the header metadata. The size is fixed during TD runtime.
> +	 */
> +	if (!tdx_get_max_quote_size(&max_size))
> +		buf_size = struct_size_t(struct tdx_quote_buf, data, max_size);
> +
> +	return buf_size;
>  }

[Severity: High]
Can this dynamic evaluation cause an out-of-bounds write or buddy allocator
corruption?

If tdx_get_max_quote_size() fails transiently during initialization when
alloc_quote_buf() is called, get_quote_buf_size() will return the default
size, and alloc_pages_exact() will allocate a buffer of that size.

If a subsequent userspace request triggers tdx_report_new_locked(), and the
size query succeeds this time returning a larger size, could the driver write
past the end of the allocation?

tdx_report_new_locked() does:

    size_t quote_buf_size = get_quote_buf_size();
    ...
    memset(quote_buf, 0, quote_buf_size);

Similarly, free_quote_buf() recalculates the size dynamically before freeing:

    size_t alloc_size = PAGE_ALIGN(get_quote_buf_size());
    ...
    free_pages_exact(buf, alloc_size);

If the evaluated size changes between allocation and free, could this cause
free_pages_exact() to free an incorrect number of pages?

Would it be safer to cache the dynamically queried allocation size once during
initialization to prevent transient failures from escalating into memory
corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930103739.2851980-1-peter.fang@intel.com?part=6