linux-kernel - Re: [PATCH v3 13/26] x86/virt/seamldr: Allocate and populate a module update request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fc3e72ec4443afd79ccade31e9e0036e645e567b.camel@intel.com>
Date: Tue, 27 Jan 2026 03:21:06 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-coco@...ts.linux.dev"
	<linux-coco@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Gao, Chao" <chao.gao@...el.com>,
	"x86@...nel.org" <x86@...nel.org>
CC: "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"kas@...nel.org" <kas@...nel.org>, "seanjc@...gle.com" <seanjc@...gle.com>,
	"Chatre, Reinette" <reinette.chatre@...el.com>, "Weiny, Ira"
	<ira.weiny@...el.com>, "tglx@...utronix.de" <tglx@...utronix.de>, "Verma,
 Vishal L" <vishal.l.verma@...el.com>, "nik.borisov@...e.com"
	<nik.borisov@...e.com>, "mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "sagis@...gle.com" <sagis@...gle.com>,
	"Chen, Farrah" <farrah.chen@...el.com>, "Duan, Zhenzhong"
	<zhenzhong.duan@...el.com>, "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
	"paulmck@...nel.org" <paulmck@...nel.org>, "Annapurve, Vishal"
	<vannapurve@...gle.com>, "yilun.xu@...ux.intel.com"
	<yilun.xu@...ux.intel.com>, "Williams, Dan J" <dan.j.williams@...el.com>,
	"bp@...en8.de" <bp@...en8.de>
Subject: Re: [PATCH v3 13/26] x86/virt/seamldr: Allocate and populate a module
 update request


> +/*
> + * Allocate and populate a seamldr_params.
> + * Note that both @module and @sig should be vmalloc'd memory.
> + */
> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
> +						   const void *sig, unsigned int sig_size)
> +{
> +	struct seamldr_params *params;
> +	const u8 *ptr;
> +	int i;
> +
> +	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!IS_ALIGNED(module_size, SZ_4K) || sig_size != SZ_4K ||
> +	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> +	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
> +		return ERR_PTR(-EINVAL);
> +

Based on the the blob format link below, we have 

struct tdx_blob
{
	...
	_u64 sigstruct[256]; // 2KB sigstruct,intel_tdx_module.so.sigstruct
	_u64 reserved2[256]; // Reserved space
	...
}

So it's clear SIGSTRUCT is just 2KB and the second half 2KB is "reserved
space".

Why is the "reserved space" treated as part of SIGSTRUCT here? 

> +
> +/*
> + * Intel TDX Module blob. Its format is defined at:
> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
> + */
> +struct tdx_blob {
> +	u16	version;
> +	u16	checksum;
> +	u32	offset_of_module;
> +	u8	signature[8];
> +	u32	len;
> +	u32	resv1;
> +	u64	resv2[509];

Nit:  Perhaps s/resv/rsvd ?

"#grep rsvd arch/x86 -Rn" gave me a bunch of results but "#grep resv" gave
me much less (and part of the results were 'resvd' and 'resv_xx' instead of
plain 'resv').
  
> +	u8	data[];
> +} __packed;

For this structure, I need to click the link and open it in a browser to
understand where is the sigstruct and module, and ...

> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> +	const struct tdx_blob *blob = (const void *)data;
> +	int module_size, sig_size;
> +	const void *sig, *module;
> +
> +	if (blob->version != 0x100) {
> +		pr_err("unsupported blob version: %x\n", blob->version);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (blob->resv1 || memchr_inv(blob->resv2, 0, sizeof(blob->resv2))) {
> +		pr_err("non-zero reserved fields\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	/* Split the given blob into a sigstruct and a module */
> +	sig		= blob->data;
> +	sig_size	= blob->offset_of_module - sizeof(struct tdx_blob);
> +	module		= data + blob->offset_of_module;
> +	module_size	= size - blob->offset_of_module;
> +

... to see whether this code makes sense.

I understand the

	...
	u64	rsvd[N*512];
	u8	module[];

is painful to be declared explicitly in 'struct tdx_blob' because IIUC we
cannot put two flexible array members at the end of the structure.

But I think if we add 'sigstruct' to the 'struct tdx_blob', e.g.,

struct tdx_blob {
	u16	version;
	...
	u64	rsvd2[509];
	u64	sigstruct[256];
	u64	rsvd3[256];
	u64	data;
} __packed;

.. we can just use

	sig		= blob->sigstruct;
	sig_size	= 2K (or 4K I don't quite follow);

which is clearer to read IMHO?

> +	return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
> +