On Tue, Apr 01, 2025 at 09:01:23AM -0400, Xiaoyao Li wrote:
> From: Isaku Yamahata <isaku.yamah...@intel.com>
>
> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
> can be provided for TDX attestation. Detailed meaning of them can be
> found:
> https://lore.kernel.org/qemu-devel/31d6dbc1-f453-4cef-ab08-4813f4e0f...@intel.com/
>
> 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.yamah...@intel.com>
> Co-developed-by: Xiaoyao Li <xiaoyao...@intel.com>
> Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
> ---
> Changes in v8:
> - it gets squashed into previous patch in v7. So split it out in v8;
>
> Changes in v6:
> - refine the doc comment of QAPI properties;
>
> Changes in v5:
> - refine the description of QAPI properties and add description of
> default value when not specified;
>
> Changes in v4:
> - describe more of there fields in qom.json
> - free the old value before set new value to avoid memory leak in
> _setter(); (Daniel)
>
> Changes in v3:
> - use base64 encoding instread of hex-string;
> ---
> qapi/qom.json | 16 +++++++-
> target/i386/kvm/tdx.c | 86 +++++++++++++++++++++++++++++++++++++++++++
> target/i386/kvm/tdx.h | 3 ++
> 3 files changed, 104 insertions(+), 1 deletion(-)
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index aa043acb1a88..77ddb2655c53 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -11,8 +11,10 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qemu/base64.h"
> #include "qapi/error.h"
> #include "qom/object_interfaces.h"
> +#include "crypto/hash.h"
>
> #include "hw/i386/x86.h"
> #include "kvm_i386.h"
> @@ -239,6 +241,7 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
> CPUX86State *env = &x86cpu->env;
> g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
> Error *local_err = NULL;
> + size_t data_len;
> int retry = 10000;
> int r = 0;
>
> @@ -250,6 +253,36 @@ 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);
>
> + if (tdx_guest->mrconfigid) {
> + g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrconfigid,
> + strlen(tdx_guest->mrconfigid), &data_len,
> errp);
> + if (!data || data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> + error_setg(errp, "TDX: failed to decode mrconfigid");
> + return -1;
> + }
When '!data', qbase64_decode will have already filled 'errp' with
details, so we must immediately 'return -1', as a repeated error_setg
call is an programming error.
The error_setg call should only be done in response to failing
the 'data_len' check and the message should specify the lengths
eg more like this
if (!data) {
return -1;
}
if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
error_setg(errp, "TDX mrconfigid len %d must match SHA384 digest len %d",
data_len, QCRYPTO_HASH_DIGEST_LEN_SHA384)
return -1;
}
> + memcpy(init_vm->mrconfigid, data, data_len);
> + }
> +
> + if (tdx_guest->mrowner) {
> + g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrowner,
> + strlen(tdx_guest->mrowner), &data_len, errp);
> + if (!data || data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> + error_setg(errp, "TDX: failed to decode mrowner");
> + return -1;
> + }
> + memcpy(init_vm->mrowner, data, data_len);
> + }
> +
> + if (tdx_guest->mrownerconfig) {
> + g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrownerconfig,
> + strlen(tdx_guest->mrownerconfig), &data_len,
> errp);
> + if (!data || data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> + error_setg(errp, "TDX: failed to decode mrownerconfig");
> + return -1;
> + }
> + memcpy(init_vm->mrownerconfig, data, data_len);
> + }
> +
> r = setup_td_guest_attributes(x86cpu, errp);
> if (r) {
> return r;
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 :|
