Re: [PATCH v3 31/70] i386/tdx: Allows mrconfigid/mrowner/mrownerconfig for TDX_INIT_VM

Daniel P. Berrangé
2023-11-15T17:32:45Z

On Wed, Nov 15, 2023 at 02:14:40AM -0500, Xiaoyao Li wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
> can be provided for TDX attestation.
>
> So far they were hard coded as 0. Now allow user to specify those values
> via property mrconfigid, mrowner and mrownerconfig. They are all in
> base64 format.
>
> example
> -object tdx-guest, \
> mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
> mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
> mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Changes in v3:
> - use base64 encoding instread of hex-string;
> ---
> qapi/qom.json | 11 +++++-
> target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
> target/i386/kvm/tdx.h | 3 ++
> 3 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 3a29659e0155..fd99aa1ff8cc 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -888,10 +888,19 @@
> # pages. Some guest OS (e.g., Linux TD guest) may require this to
> # be set, otherwise they refuse to boot.
> #
> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
> +#
> +# @mrowner: base64 encoded MROWNER SHA384 digest
> +#
> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
> +#
> # Since: 8.2
> ##
> { 'struct': 'TdxGuestProperties',
> - 'data': { '*sept-ve-disable': 'bool' } }
> + 'data': { '*sept-ve-disable': 'bool',
> + '*mrconfigid': 'str',
> + '*mrowner': 'str',
> + '*mrownerconfig': 'str' } }
>
> ##
> # @ThreadContextProperties:
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 28b3c2765c86..b70efbcab738 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -13,6 +13,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qemu/base64.h"
> #include "qapi/error.h"
> #include "qom/object_interfaces.h"
> #include "standard-headers/asm-x86/kvm_para.h"
> @@ -508,6 +509,8 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
> X86CPU *x86cpu = X86_CPU(cpu);
> CPUX86State *env = &x86cpu->env;
> struct kvm_tdx_init_vm *init_vm;
> + uint8_t *data;
> + size_t data_len;

Don't declare these here.

> int r = 0;
>
> qemu_mutex_lock(&tdx_guest->lock);
> @@ -518,6 +521,38 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
> init_vm = g_malloc0(sizeof(struct kvm_tdx_init_vm) +
> sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES);
>
> +#define SHA384_DIGEST_SIZE 48
> +
> + if (tdx_guest->mrconfigid) {

> + data = qbase64_decode(tdx_guest->mrconfigid,
> + strlen(tdx_guest->mrconfigid), &data_len, errp);

Declare it here:

g_autofree uint8_t *data = qbase64_decode(...)

so we aviod the memory leak of 'data' in each if()... block

> + if (!data || data_len != SHA384_DIGEST_SIZE) {
> + error_setg(errp, "TDX: failed to decode mrconfigid");
> + return -1;
> + }
> + memcpy(init_vm->mrconfigid, data, data_len);
> + }
> +
> + if (tdx_guest->mrowner) {
> + data = qbase64_decode(tdx_guest->mrowner,
> + strlen(tdx_guest->mrowner), &data_len, errp);
> + if (!data || data_len != SHA384_DIGEST_SIZE) {
> + error_setg(errp, "TDX: failed to decode mrowner");
> + return -1;
> + }
> + memcpy(init_vm->mrowner, data, data_len);
> + }
> +
> + if (tdx_guest->mrownerconfig) {
> + data = qbase64_decode(tdx_guest->mrownerconfig,
> + strlen(tdx_guest->mrownerconfig), &data_len, errp);
> + if (!data || data_len != SHA384_DIGEST_SIZE) {
> + error_setg(errp, "TDX: failed to decode mrownerconfig");
> + return -1;
> + }
> + memcpy(init_vm->mrownerconfig, data, data_len);
> + }
> +
> r = kvm_vm_enable_cap(kvm_state, KVM_CAP_MAX_VCPUS, 0, ms->smp.cpus);
> if (r < 0) {
> error_setg(errp, "Unable to set MAX VCPUS to %d", ms->smp.cpus);
> @@ -567,6 +602,48 @@ static void tdx_guest_set_sept_ve_disable(Object *obj, bool value, Error **errp)
> }
> }
> +static void tdx_guest_set_mrconfigid(Object *obj, const char *value, Error **errp)
> +{
> + TdxGuest *tdx = TDX_GUEST(obj);
> +
> + tdx->mrconfigid = g_strdup(value);
> +}

g_free(tdx->mrconfigid) first to be sure we don't leak if
the value is set twice.

> +
> +static char * tdx_guest_get_mrowner(Object *obj, Error **errp)
> +{
> + TdxGuest *tdx = TDX_GUEST(obj);
> +
> + return g_strdup(tdx->mrowner);
> +}
> +
> +static void tdx_guest_set_mrowner(Object *obj, const char *value, Error **errp)
> +{
> + TdxGuest *tdx = TDX_GUEST(obj);
> +
> + tdx->mrconfigid = g_strdup(value);
> +}
> +
> +static char * tdx_guest_get_mrownerconfig(Object *obj, Error **errp)
> +{
> + TdxGuest *tdx = TDX_GUEST(obj);
> +
> + return g_strdup(tdx->mrownerconfig);
> +}
> +
> +static void tdx_guest_set_mrownerconfig(Object *obj, const char *value, Error **errp)
> +{
> + TdxGuest *tdx = TDX_GUEST(obj);
> +
> + tdx->mrconfigid = g_strdup(value);
> +}
> +
> /* tdx guest */
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
> tdx_guest,
> @@ -586,6 +663,14 @@ static void tdx_guest_init(Object *obj)
> object_property_add_bool(obj, "sept-ve-disable",
> tdx_guest_get_sept_ve_disable,
> tdx_guest_set_sept_ve_disable);
> + object_property_add_str(obj, "mrconfigid",
> + tdx_guest_get_mrconfigid,
> + tdx_guest_set_mrconfigid);
> + object_property_add_str(obj, "mrowner",
> + tdx_guest_get_mrowner, tdx_guest_set_mrowner);
> + object_property_add_str(obj, "mrownerconfig",
> + tdx_guest_get_mrownerconfig,
> + tdx_guest_set_mrownerconfig);
> }
>
> static void tdx_guest_finalize(Object *obj)
> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> index 432077723ac5..6e39ef3bac13 100644
> --- a/target/i386/kvm/tdx.h
> +++ b/target/i386/kvm/tdx.h
> @@ -21,6 +21,9 @@ typedef struct TdxGuest {
>
> bool initialized;
> uint64_t attributes; /* TD attributes */
> + char *mrconfigid; /* base64 encoded sha348 digest */
> + char *mrowner; /* base64 encoded sha348 digest */
> + char *mrownerconfig; /* base64 encoded sha348 digest */
> } TdxGuest;
>
> #ifdef CONFIG_TDX
> --
> 2.34.1
>

With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|