Re: [PATCH v3 4/4] virt: tdx-guest: Allocate Quote buffer dynamically

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Untrusted VMM input `TDCS_QUOTE_MAX_SIZE` is passed directly to the page allocator without bounding and without `__GFP_NOWARN`, which can trigger a kernel warning (`WARN_ON_ONCE`) and panic the guest if `panic_on_warn` is set.

Pre-existing issues:
- [High] Pre-existing UAF/Use-After-Unload race condition if module initialization fails after registering the misc device.
--

--- Patch [4]: [PATCH v3 4/4] virt: tdx-guest: Allocate Quote buffer dynamically ---
commit 25802a377289c67ad6c07c5b41f81b3b9c799fbe
Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

virt: tdx-guest: Allocate Quote buffer dynamically

This commit modifies the TDX guest driver to dynamically allocate the Quote
buffer based on a size reported by the TDX module, replacing a fixed 128KB
allocation. It removes the __GFP_NOWARN flag to ensure allocation failures
produce a warning.

> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 3d3f79ab45af0..8919b1a1154e7 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c

[ ... ]

> @@ -222,11 +222,31 @@ static void free_quote_buf(void *buf, size_t len)
>  	free_pages_exact(buf, len);
>  }
>  
> +/* Return a buffer size large enough to hold a Quote */
> +static size_t get_quote_buf_size(void)
> +{
> +	u32 quote_size = tdx_get_max_quote_size();
> +
> +	/*
> +	 * Older TDX modules do not report a maximum Quote size, so use
> +	 * the default.
> +	 */
> +	if (!quote_size)
> +		return GET_QUOTE_DEFAULT_BUF_SIZE;
> +
> +	/* The reported size does not include the buffer header */
> +	return PAGE_ALIGN(TDX_QUOTE_BUF_LEN(quote_size));
> +}
> +
>  static void *alloc_quote_buf(size_t len)
>  {
>  	unsigned int count = len >> PAGE_SHIFT;
>  	void *addr;
>  
> +	/*
> +	 * This fails if the requested size exceeds the buddy allocator's
> +	 * maximum order (order-10, 4MB).
> +	 */
>  	addr = alloc_pages_exact(len, GFP_KERNEL | __GFP_ZERO);

[Severity: High]
Does this allow an untrusted VMM to trigger a guest panic?

Since tdx_get_max_quote_size() reads the TDCS_QUOTE_MAX_SIZE metadata field,
an untrusted host could supply a value that causes get_quote_buf_size() to
return a length where get_order(size) > MAX_PAGE_ORDER.

Because the __GFP_NOWARN flag was removed, passing this unvalidated size to
alloc_pages_exact() will trigger a WARN_ON_ONCE inside the page allocator.

If the guest boots with panic_on_warn=1, this gives the VMM a trivial denial
of service.

Should the calculated size be clamped to a safe maximum before calling
alloc_pages_exact(), or should the __GFP_NOWARN flag be retained?

>  	if (!addr)
>  		return NULL;

[ ... ]

> @@ -416,7 +436,7 @@ static int __init tdx_guest_init(void)
>  	if (ret)
>  		goto deinit_mr;
>  
> -	quote_data_len = GET_QUOTE_BUF_SIZE;
> +	quote_data_len = get_quote_buf_size();
>  	quote_data = alloc_quote_buf(quote_data_len);
>  	if (!quote_data) {
>  		pr_err("Failed to allocate Quote buffer\n");

[Severity: High]
This is a pre-existing issue, but does this allocation failure path leave a
window for a use-after-free or use-after-unload race?

The tdx_guest_init() function calls misc_register() just before attempting to
allocate the quote buffer.

If alloc_quote_buf() fails, it will jump to the error path and call
misc_deregister().

However, misc_deregister() does not revoke active open file descriptors.
If a concurrent userspace process opens the misc device between registration
and the allocation failure, it can execute ioctls after the module
initialization fails.

This can result in accessing freed memory or executing unloaded code.

Since alloc_quote_buf() can now attempt larger allocations up to 4MB, it
seems much more likely to fail.

Should misc_register() be moved to the end of the initialization sequence to
prevent userspace from accessing a partially initialized device?

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