| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <419a166c-a4a8-46ad-a7ed-4b8ec23ca7d4@intel.com>
Date: Fri, 6 Dec 2024 16:37:14 +0800
From: Xiaoyao Li <xiaoyao.li@...el.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>, pbonzini@...hat.com,
seanjc@...gle.com
Cc: yan.y.zhao@...el.com, isaku.yamahata@...il.com, kai.huang@...el.com,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
tony.lindgren@...ux.intel.com, reinette.chatre@...el.com
Subject: Re: [PATCH v2 03/25] x86/virt/tdx: Read essential global metadata for
KVM
On 10/31/2024 3:00 AM, Rick Edgecombe wrote:
> From: Kai Huang <kai.huang@...el.com>
>
> KVM needs two classes of global metadata to create and run TDX guests:
>
> - "TD Control Structures"
> - "TD Configurability"
>
> The first class contains the sizes of TDX guest per-VM and per-vCPU
> control structures. KVM will need to use them to allocate enough space
> for those control structures.
>
> The second class contains info which reports things like which features
> are configurable to TDX guest etc. KVM will need to use them to
> properly configure TDX guests.
>
> Read them for KVM TDX to use.
>
> The code change is auto-generated by re-running the script in [1] after
> uncommenting the "td_conf" and "td_ctrl" part to regenerate the
> tdx_global_metadata.{hc} and update them to the existing ones in the
> kernel.
>
> #python tdx.py global_metadata.json tdx_global_metadata.h \
> tdx_global_metadata.c
>
> The 'global_metadata.json' can be fetched from [2].
>
> Link: https://lore.kernel.org/kvm/0853b155ec9aac09c594caa60914ed6ea4dc0a71.camel@intel.com/ [1]
> Link: https://cdrdv2.intel.com/v1/dl/getContent/795381 [2]
> Signed-off-by: Kai Huang <kai.huang@...el.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> ---
> uAPI breakout v2:
> - New patch
> ---
> arch/x86/include/asm/tdx_global_metadata.h | 19 +++++++++
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 46 +++++++++++++++++++++
> 2 files changed, 65 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index fde370b855f1..206090c9952f 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -32,11 +32,30 @@ struct tdx_sys_info_cmr {
> u64 cmr_size[32];
> };
>
> +struct tdx_sys_info_td_ctrl {
> + u16 tdr_base_size;
> + u16 tdcs_base_size;
> + u16 tdvps_base_size;
> +};
> +
> +struct tdx_sys_info_td_conf {
> + u64 attributes_fixed0;
> + u64 attributes_fixed1;
> + u64 xfam_fixed0;
> + u64 xfam_fixed1;
> + u16 num_cpuid_config;
> + u16 max_vcpus_per_td;
> + u64 cpuid_config_leaves[32];
> + u64 cpuid_config_values[32][2];
> +};
> +
> struct tdx_sys_info {
> struct tdx_sys_info_version version;
> struct tdx_sys_info_features features;
> struct tdx_sys_info_tdmr tdmr;
> struct tdx_sys_info_cmr cmr;
> + struct tdx_sys_info_td_ctrl td_ctrl;
> + struct tdx_sys_info_td_conf td_conf;
> };
>
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 2fe57e084453..44c2b3e079de 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -76,6 +76,50 @@ static int get_tdx_sys_info_cmr(struct tdx_sys_info_cmr *sysinfo_cmr)
> return ret;
> }
>
> +static int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
> +{
> + int ret = 0;
> + u64 val;
> +
> + if (!ret && !(ret = read_sys_metadata_field(0x9800000100000000, &val)))
> + sysinfo_td_ctrl->tdr_base_size = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x9800000100000100, &val)))
> + sysinfo_td_ctrl->tdcs_base_size = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x9800000100000200, &val)))
> + sysinfo_td_ctrl->tdvps_base_size = val;
> +
> + return ret;
> +}
> +
> +static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
> +{
> + int ret = 0;
> + u64 val;
> + int i, j;
> +
> + if (!ret && !(ret = read_sys_metadata_field(0x1900000300000000, &val)))
> + sysinfo_td_conf->attributes_fixed0 = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x1900000300000001, &val)))
> + sysinfo_td_conf->attributes_fixed1 = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x1900000300000002, &val)))
> + sysinfo_td_conf->xfam_fixed0 = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x1900000300000003, &val)))
> + sysinfo_td_conf->xfam_fixed1 = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x9900000100000004, &val)))
> + sysinfo_td_conf->num_cpuid_config = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x9900000100000008, &val)))
> + sysinfo_td_conf->max_vcpus_per_td = val;
> + for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
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.
> + if (!ret && !(ret = read_sys_metadata_field(0x9900000300000400 + i, &val)))
> + sysinfo_td_conf->cpuid_config_leaves[i] = val;
> + for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
> + for (j = 0; j < 2; j++)
> + if (!ret && !(ret = read_sys_metadata_field(0x9900000300000500 + i * 2 + j, &val)))
> + sysinfo_td_conf->cpuid_config_values[i][j] = val;
> +
> + return ret;
> +}
> +
> static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> {
> int ret = 0;
> @@ -84,6 +128,8 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
> ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
> ret = ret ?: get_tdx_sys_info_cmr(&sysinfo->cmr);
> + ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> + ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>
> return ret;
> }
Powered by blists - more mailing lists