Re: [PATCH v9 11/23] x86/virt/seamldr: Allocate and populate a module update request

[Chao Gao]

>+static int init_seamldr_params(struct seamldr_params *params, const u8 *data, u32 size)
>+{
>+	const struct tdx_image *image		= (const void *)data;
>+	const struct tdx_image_header *header	= &image->header;
>+
>+	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
>+	u32 module_len		= header->module_nr_pages * PAGE_SIZE;

I looked at Sashiko's two reports here.

 (1) The header is dereferenced before validating that the input is large
     enough to contain a full header.

 (2) The page-count to byte-count multiplication could in principle
     overflow.

For (1), I agree the validation order should be fixed. Even if the input
buffer is page-backed in practice, the parser should still verify that
size is at least sizeof(struct tdx_image_header) before dereferencing the
header.

For (2), I think using u64 for the derived byte lengths is sufficient in
this case. That avoids overflow in the multiplication itself, and the later
size consistency check:

	HEADER_SIZE + sigstruct_len + module_len != size

will reject malformed inputs.

Below is the fix I plan to fold into this patch in the next revision:

diff --git a/arch/x86/virt/vmx/tdx/seamldr.c
b/arch/x86/virt/vmx/tdx/seamldr.c
index 58ce39315b60..9f4350079477 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -148,8 +148,8 @@ static int init_seamldr_params(struct seamldr_params
*params, const u8 *data, u3
	const struct tdx_image *image		= (const void *)data;
	const struct tdx_image_header *header	= &image->header;
 
-	u32 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
-	u32 module_len		= header->module_nr_pages * PAGE_SIZE;
+	u64 sigstruct_len	= header->sigstruct_nr_pages * PAGE_SIZE;
+	u64 module_len		= header->module_nr_pages * PAGE_SIZE;
 
	u8 *header_start	= (u8 *)header;
	u8 *header_end		= header_start + HEADER_SIZE;
@@ -299,6 +299,9 @@ int seamldr_install_module(const u8 *data, u32 size)
	struct seamldr_params *params;
	int ret;
 
+	if (size <= HEADER_SIZE)
+		return -EINVAL;
+
	params = kzalloc_obj(*params);
	if (!params)
		return -ENOMEM;