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 :-)
