Re: [PATCH v2 04/17] x86/virt/tdx: Add extra memory to TDX module for the extensions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Hardware PFN mismatch due to unaligned slab allocation for `hpa_list` with debugging enabled.
- [Medium] Resource leak of un-added contiguous host memory on TDX module error path.
- [Medium] Improper use of node_online_map instead of N_MEMORY node states for allocation.
- [Low] Unnecessary early boot memory allocation error handling returning -ENOMEM.
--

commit e910725d57b782da4b05f3bb30b4df0fa4da1da3
Author: Xu Yilun <yilun.xu@linux.intel.com>

x86/virt/tdx: Add extra memory to TDX module for the extensions

This patch introduces a memory feeding process via SEAMCALL TDH.EXT.MEM.ADD
to allocate one-time memory for TDX module extensions during initialization.
It allocates contiguous memory and passes it to the module using a newly
defined HPA_LIST_INFO format.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 6f3596f11d250..dab17822c1c69 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c

[ ... ]

> +static __init int tdx_ext_mem_setup(unsigned int required_pages)
> +{
> +	struct tdx_hpa_list *hpa_list;
> +	struct page *page;
> +	unsigned int i;
> +	int ret;
> +
> +	/*
> +	 * memory_pool_required_pages == 0 means no need to add pages,
> +	 * skip the memory setup.
> +	 */
> +	if (!required_pages)
> +		return 0;
> +
> +	hpa_list = kzalloc_obj(*hpa_list);

[Severity: High]
Does this code pass unaligned physical pages to the hardware if slab
debugging is enabled?

When CONFIG_SLUB_DEBUG or KASAN is enabled, slab objects are padded with
metadata, causing the object to start at a non-zero offset within a physical
page. The virt_to_page(hpa_list) call below will grab the base page frame
which the hardware will read from offset 0, leading it to consume SLUB
metadata instead of the valid hpa_list entries.

Should this allocation use the page allocator directly (e.g.,
__get_free_page()) to guarantee strict page alignment?

> +	if (!hpa_list)
> +		return -ENOMEM;
> +
> +	page = alloc_contig_pages(required_pages, GFP_KERNEL, numa_mem_id(),
> +				  &node_online_map);

[Severity: Medium]
Can this allocation fall back to memoryless nodes?

The node_online_map can include nodes without memory. The mm allocation
guidelines require using the N_MEMORY state mask (e.g., node_states[N_MEMORY])
to correctly constrain memory allocations to nodes that actually have memory.

> +	if (!page) {
> +		ret = -ENOMEM;

[Severity: Low]
Is this complex error handling necessary here?

Since tdx_ext_mem_setup() is only called from the __init path, physical memory
should be available. If an allocation of this size fails during early boot,
the system likely cannot boot anyway, making graceful error cleanup here
largely dead code.

> +		goto out_free_hpa_list;
> +	}
> +
> +	i = 0;
> +	while (i < required_pages) {
> +		unsigned int nents = min(required_pages - i,
> +					 ARRAY_SIZE(hpa_list->phys));
> +		unsigned int j;
> +
> +		for (j = 0; j < nents; j++)
> +			hpa_list->phys[j] = page_to_phys(page + i + j);
> +
> +		ret = tdx_ext_mem_add(virt_to_page(hpa_list), nents);
> +		/*
> +		 * No SEAMCALLs to reclaim the added pages. For simple error
> +		 * handling, leak all pages.
> +		 */
> +		WARN(ret, "Fatal: TDX module rejected (%d) memory for extensions, stranded all pages\n",
> +		     ret);
> +		if (ret)
> +			break;

[Severity: Medium]
Does this intentionally leak un-added host memory on the error path?

While pages that have already been accepted by the hardware cannot be
reclaimed, the remaining contiguous block of pages (from page + i to the end)
are still entirely host-owned. Could these remaining un-added pages be safely
freed via free_contig_range() instead of permanently leaking tens of megabytes?

> +
> +		i += nents;
> +	}

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