Re: [PATCH v2 05/17] x86/virt/tdx: Make TDX module initialize the extensions

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] tdx_ext_init() (added in this patch) and tdx_ext_mem_add() silently discard the 64-bit hardware error code from their SEAMCALLs and return -EFAULT without logging.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but by the earlier commit 'x86/virt/tdx: Add extra memory to TDX module for the extensions'. The `hpa_list` structure is allocated via `kzalloc_obj()` which does not guarantee page-alignment.
--

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

x86/virt/tdx: Make TDX module initialize the extensions
    
After providing all required memory to TDX module, initialize TDX
module extensions via TDH.EXT.INIT, so extension SEAMCALLs can be used.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index dab17822c1c69..900928de373ae 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
[ ... ]
> @@ -1167,6 +1167,22 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
>  	return 0;
>  }
>  
> +/* Initialize TDX module extensions for extension SEAMCALLs */
> +static __init int tdx_ext_init(void)
> +{
> +	struct tdx_module_args args = {};
> +	u64 r;
> +
> +	do {
> +		r = seamcall(TDH_EXT_INIT, &args);
> +	} while (r == TDX_INTERRUPTED_RESUMABLE);
> +
> +	if (r != TDX_SUCCESS)
> +		return -EFAULT;

[Severity: Medium]
Does this code silently discard the 64-bit hardware error code?

When TDH_EXT_INIT fails, replacing the return value with -EFAULT means
the caller only logs a generic initialization failure without details.
Other TDX initialization functions use seamcall_prerr_ret() to preserve
and log the hardware error.

Could this be updated to use seamcall_prerr_ret() to ensure the exact
firmware failure reason is logged?

> +
> +	return 0;
> +}
> +
[ ... ]
> @@ -1285,7 +1301,11 @@ static __init int init_tdx_module_extensions(void)
>  	if (!sysinfo_ext.ext_required)
>  		return 0;
>  
> -	return tdx_ext_mem_setup(sysinfo_ext.memory_pool_required_pages);
> +	ret = tdx_ext_mem_setup(sysinfo_ext.memory_pool_required_pages);

[Severity: High]
This isn't a bug introduced by this patch, but does the earlier
introduction of tdx_ext_mem_setup() cause an alignment issue?

Looking at tdx_ext_mem_setup() in arch/x86/virt/vmx/tdx/tdx.c:

tdx_ext_mem_setup() {
    ...
    hpa_list = kzalloc_obj(*hpa_list);
    if (!hpa_list)
        return -ENOMEM;
    ...
    ret = tdx_ext_mem_add(virt_to_page(hpa_list), nents);
    ...
}

The TDX module's TDH.EXT.MEM.ADD leaf expects a page-aligned PFN for the
HPA list. Since kmalloc does not guarantee page-alignment (especially if
CONFIG_SLUB_DEBUG is enabled), virt_to_page() drops the unaligned offset.

Will this result in the TDX module reading from offset 0 of the page,
potentially consuming SLUB metadata instead of the actual hpa_list
entries?

> +	if (ret)
> +		return ret;
> +
> +	return tdx_ext_init();
>  }

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