Re: [PATCH v2 03/25] x86/virt/tdx: Read essential global metadata for KVM

["Huang, Kai" <kai.huang@intel.com>]

On 12/12/2024 8:31 am, Edgecombe, Rick P wrote:
> On Sat, 2024-12-07 at 00:00 +0000, Huang, Kai wrote:
>>> On 12/6/24 08:13, Huang, Kai wrote:
>>>> It is not safe. We need to check
>>>>
>>>>         sysinfo_td_conf->num_cpuid_config <= 32.
>>>>
>>>> If the TDX module version is not matched with the json file that was
>>>> used to generate the tdx_global_metadata.h, the num_cpuid_config
>>>> reported by the actual TDX module might exceed 32 which causes
>>>> out-of-bound array access.
>>>
>>> The JSON *IS* the ABI description. It can't change between versions of the
>>> TDX module. It can only be extended. The "32" is not in the spec because the
>>> spec refers to the JSON!
>>
>> Ah, yeah, agreed, the "spec refers to the JSON".  :-)
> 
> So we heard back from TDX module folks that they were thinking the 32 could
> change to be larger (thanks Kai for checking). We need to continue education
> with them around what KVM is depending on as TDX Module ABI. And we should get
> something clearer than these JSONs.
> 
> But in the meantime, we could tell TDX module team they need an opt-in to change
> this field. We could also add an actual check to fail cleanly:
> 

Hi Paolo/Sean/Dave,

TDX module team has acked changing 32 to a higher value in future 
modules is a breaking of ABI.  They also promised 128 is the maximum 
value they reserved for CPUID_CONFIGs thus won't change for all modules. 
  They will update the JSON to address.

I just send out an updated v2.1 of this patch to bump array size for 
CPUID_CONFIGs to 128 and add paranoid checks to protect kernel from 
potential TDX module breakage on this.

Appreciate if you can help to review, but for now, wish you have a 
wonderful Christmas :-)