Re: [RFC PATCH v2 10/10] x86/virt/tdx: Verify structure member sizes against metadata field IDs

[Chao Gao]

On Thu, Sep 24, 2026 at 06:47:19AM +0800, Edgecombe, Rick P wrote:
>On Fri, 2026-09-18 at 06:29 -0700, Chao Gao wrote:
>> A metadata field ID encodes the size of a single element. TDX_SYSINFO_MAP()
>> instead derives the copy size from the destination member, and nothing
>> verifies that the two sizes agree.
>> 
>> A wrongly typed member is a kernel bug: declaring a u32 for an 8-byte
>> metadata field would silently store only its low 4 bytes.
>> 
>> Add macros to extract the element size encoded in a field ID and verify it
>> against the destination member size at build time.
>
>The two things we could do are extract the field code and check it, or add it
>into the field automatically from the struct size. In the later case the field
>id's would be specified without the size bits already filled in. But since the
>metadata docs have the field code already embedded when they are listed in the
>docs, that is the most natural and easy thing to add to the field id code. It
>makes it easy to search the docs too. So the checking design gives us some extra
>safety, make it easier to add the code and search the docs.

I'll explain in the changelog why we extract the size from the field ID
and check it, rather than building the field ID from the member size.

>
>I think probably you need to explain a bit more about what and why the fieldid
>size bits exist, but I agree with the design.

Sure. I will add:

  TDH.SYS.RD returns every field's value as a u64, but metadata fields
  aren't all 64 bits wide. A field ID encodes the size of one field
  (bits 33:32), so the ID alone gives the field's width.

As for why the size bits exist at all, from the TDX module source it
looks like the module itself uses them: it stores array fields packed
in memory and uses the size to locate each element (offset = index * size).
The kernel doesn't need them except for the size check in this patch.

>
>> +/*
>> + * Sub-field definitions of TDX global metadata field IDs.
>> + *
>> + * See "Metadata Field Identifier" in the Intel TDX Module ABI
>> + * Specification.
>> + *
>> + *  - Bit 33:32: ELEMENT_SIZE_CODE -- log2 of a single metadata
>> + *                                    element's size in bytes
>> + */
>> +#define TDX_MD_FIELD_ELE_SIZE_CODE(field_id)	\
>> +	(((field_id) & GENMASK_ULL(33, 32)) >> 32)
>> +
>> +#define TDX_MD_FIELD_ELE_SIZE(field_id)	\
>> +	(1 << TDX_MD_FIELD_ELE_SIZE_CODE(field_id))
>> +
>
>I'd think these could be squashed together since only TDX_MD_FIELD_ELE_SIZE gets
>used anywhere else.

Sure. Will do.