Re: [RFC PATCH v1 1/3] x86/virt/seamldr: Allocate 'struct seamldr_params' after image validation

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Fri, 2026-10-30 at 01:52 -0700, Chao Gao wrote:
> 'struct seamldr_params' is the in-memory ABI the kernel passes to P-SEAMLDR
> to update the TDX module. It has a fixed size and is allocated before the
> update image is validated.
> 
> To support larger TDX modules, the ABI was extended to let
> 'struct seamldr_params' link to extra pages. The number of extra pages
> depends on the module page count, which can be trusted only after the image
> header is validated.
> 
> Move the allocation after the image validation in init_seamldr_params(),
> so a later change can allocate 'struct seamldr_params' and the extra
> pages as one buffer. Rename the function to build_seamldr_params() since
> it now allocates and returns a buffer rather than filling in one the
> caller provides.
> 
> No functional change intended.

So we want to have the allocation happen after header->module_nr_pages is
verified so we don't allocate some ridiculous size. And the verification
currently happens all in one place. So move the allocation.

Did you look at just moving the one header field check earlier? Wondering if it
requires less churn.

Hmm, also, this is an admin only thing right? What goes wrong if the admin user
passes a huge value?

In any case, I think the log could be simpler and more plain while still
explaining what is happening better.

> 
> AI was used to review the code and workshop the logs. It suggested renaming
> init_seamldr_params() and updating the comments above it and at its call
> site.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 40 ++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b1137ca6150d..3513fd28536a 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -157,13 +157,14 @@ static void populate_seamldr_params(struct
> seamldr_params *params,
>  
>  /*
>   * @image points to a vmalloc()'d 'struct tdx_image'. Transform
> - * it into @params which is the P-SEAMLDR ABI format.
> + * it into a newly allocated 'struct seamldr_params', which is
> + * the P-SEAMLDR ABI format.
>   */
> -static int init_seamldr_params(struct seamldr_params *params,
> -			       const struct tdx_image *image,
> -			       u32 image_len)
> +static struct seamldr_params *build_seamldr_params(const struct tdx_image
> *image,
> +						   u32 image_len)
>  {
>  	const struct tdx_image_header *header	= &image->header;
> +	struct seamldr_params *params;
>  
>  	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
>  	u32 module_len		= header->module_nr_pages    * PAGE_SIZE;
> @@ -178,25 +179,29 @@ static int init_seamldr_params(struct seamldr_params
> *params,
>  
>  	/* Check the calculated payload size against the image size. */
>  	if (TDX_IMAGE_HEADER_SIZE + sigstruct_len + module_len != image_len)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	/* Reject unsupported tdx_image ABI versions. */
>  	if (header->version != TDX_IMAGE_VERSION_2)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (header->sigstruct_nr_pages > SEAMLDR_MAX_NR_SIG_PAGES ||
>  	    header->module_nr_pages    > SEAMLDR_MAX_NR_MODULE_PAGES)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (memcmp(header->signature, "TDX-BLOB", sizeof(header->signature)))
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (memchr_inv(header->reserved, 0, sizeof(header->reserved)))
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
> +
> +	params = kzalloc_obj(*params);
> +	if (!params)
> +		return ERR_PTR(-ENOMEM);
>  
>  	populate_seamldr_params(params, sigstruct_start, header-
> >sigstruct_nr_pages,
>  					module_start,    header-
> >module_nr_pages);
> -	return 0;
> +	return params;
>  }
>  
>  /*
> @@ -321,7 +326,7 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  	int ret;
>  
>  	/*
> -	 * init_seamldr_params() reads the header early.
> +	 * build_seamldr_params() reads the header early.
>  	 * Ensure there is enough data to do at least that.
>  	 */
>  	if (data_len < TDX_IMAGE_HEADER_SIZE)
> @@ -329,14 +334,10 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  
>  	image = (const struct tdx_image *)data;
>  
> -	params = kzalloc_obj(*params);
> -	if (!params)
> -		return -ENOMEM;
> -
> -	/* Populate 'params' from 'image'. */
> -	ret = init_seamldr_params(params, image, data_len);
> -	if (ret)
> -		goto out;
> +	/* Build 'params' from 'image'. */
> +	params = build_seamldr_params(image, data_len);
> +	if (IS_ERR(params))
> +		return PTR_ERR(params);
>  
>  	/* Ensure a stable set of online CPUs for the update process. */
>  	cpus_read_lock();
> @@ -345,7 +346,6 @@ int seamldr_install_module(const u8 *data, u32 data_len)
>  				      cpu_online_mask);
>  	cpus_read_unlock();
>  
> -out:
>  	kfree(params);
>  	return ret;
>  }