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

[Binbin Wu]

On 10/12/2026 5:47 PM, Chao Gao wrote:
> The TDX module reports its capabilities and limits as a set of metadata
> fields, each identified by a field ID, and TDH.SYS.RD reads one field by
> its ID. TDH.SYS.RD returns the value as a u64, but metadata fields are not
> all 64 bits wide. A field ID therefore encodes the field's size into
> bits 33:32.
> 
> The kernel mirrors these fields in C structures, and each member's size
> must match the size encoded in the field ID. FIELD_MAP() stores the member
> size and uses it to decide how many bytes to copy from the value returned
> by TDH.SYS.RD. The size in the field ID is never consulted, so any mismatch
> goes unnoticed. Declaring a u32 member for a 64-bit field, for example,
> would silently store only its low 4 bytes.
> 
> Add a build-time check that the size encoded in each field ID matches the
> size of the corresponding C structure member. A wrongly typed member then
> fails the build with no runtime cost.
> 
> BUILD_BUG_ON() cannot be used in a structure initializer, so use
> BUILD_BUG_ON_ZERO() and add its zero result to the .size initializer. This
> performs the build-time check without changing the stored size.
> 
> An alternative would be to leave the size bits out of the field ID
> definitions and construct the IDs from the member sizes, which makes a
> mismatch impossible. But the TDX module ABI definitions list the full field
> IDs, and definitions with the size bits stripped would match nothing in the
> docs, making them harder to verify. Keep the IDs exactly as documented and
> check the size they encode against the C type instead.
> 
> AI was used under supervision to review code and workshop logs. It
> suggested moving the size check into its own macro to keep the .size line
> short. Rick also used AI to write TDX_FIELD_SIZE_CODE(), which converts a
> member size to the size encoding in the field ID.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>

Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>

One nit below.

[...]

>  
> +/*
> + * The size encoded in the field ID and the size of the destination C
> + * member must agree.
> + */
> +#define TDX_FIELD_SIZE_CHECK(_field, _type, _member)		\
> +	BUILD_BUG_ON_ZERO(((_field) & TDX_FIELD_SIZE_MASK) !=	\
> +		TDX_FIELD_SIZE_CODE(sizeof_field(_type, _member)))
> +
>  #define __FIELD_MAP(_field, _type, _member)			\
>  {								\
>  	.field_id	= _field,				\
>  	.offset		= offsetof(_type, _member),		\
> -	.size		= sizeof_field(_type, _member),		\
> +	.size		= sizeof_field(_type, _member) +	\
> +			  TDX_FIELD_SIZE_CHECK(			\
> +				_field, _type, _member),	\
>  }
>  

Nit:

This doesn't cover the TDX_FIELD_CPUID_CONFIG_LEAVES and
TDX_FIELD_CPUID_CONFIG_VALUES.

Not sure it's worth to add static_assert() in get_tdx_sys_info_td_conf(). But if
not, maybe it can be mentioned it in the changelog.