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

[Chao Gao <chao.gao@intel.com>]

On Tue, Oct 13, 2026 at 07:10:36PM +0800, Binbin Wu wrote:
>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@intel.com>
>
>Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

Thanks!

>> +/*
>> + * 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.

Yes.

>
>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.

I'll add checks in get_tdx_sys_info_td_conf():

	(void)TDX_FIELD_SIZE_CHECK(TDX_FIELD_CPUID_CONFIG_LEAVES,
				   struct tdx_sys_info_td_conf,
				   cpuid_config_leaves[0]);

	(void)TDX_FIELD_SIZE_CHECK(TDX_FIELD_CPUID_CONFIG_VALUES,
				   struct tdx_sys_info_td_conf,
				   cpuid_config_values[0][0]);


TDX_FIELD_SIZE_CHECK() uses BUILD_BUG_ON_ZERO() so it can be used in
structure initializers. The (void) casts avoid unused-value warnings
when used as standalone statements.