TDX: Fix and enhancement

[PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

Posted by Xiaoyao Li 5 months, 2 weeks ago

The error message is misleading - we successfully decoded the data,
the decoded data was simply with the wrong length.

Change the error message to show it is an length check failure with both
the received and expected values.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
---
 target/i386/kvm/tdx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 17e671f1710f..4172bbd23340 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -1026,7 +1026,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrconfigid");
+            error_setg(errp, "TDX 'mrconfigid' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrconfigid, data, data_len);
@@ -1039,7 +1041,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrowner");
+            error_setg(errp, "TDX 'mrowner' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrowner, data, data_len);
@@ -1052,7 +1056,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrownerconfig");
+            error_setg(errp, "TDX 'mrownerconfig' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrownerconfig, data, data_len);
-- 
2.43.0

Re: [PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

Posted by Igor Mammedov 5 months, 2 weeks ago

On Tue,  3 Jun 2025 01:03:05 -0400
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> The error message is misleading - we successfully decoded the data,
> the decoded data was simply with the wrong length.
> 
> Change the error message to show it is an length check failure with both
> the received and expected values.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
> ---
>  target/i386/kvm/tdx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 17e671f1710f..4172bbd23340 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -1026,7 +1026,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrconfigid");
> +            error_setg(errp, "TDX 'mrconfigid' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrconfigid, data, data_len);
> @@ -1039,7 +1041,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrowner");
> +            error_setg(errp, "TDX 'mrowner' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrowner, data, data_len);
> @@ -1052,7 +1056,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrownerconfig");
> +            error_setg(errp, "TDX 'mrownerconfig' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrownerconfig, data, data_len);

Re: [PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

Posted by Daniel P. Berrangé 5 months, 2 weeks ago

On Tue, Jun 03, 2025 at 01:03:05AM -0400, Xiaoyao Li wrote:
> The error message is misleading - we successfully decoded the data,
> the decoded data was simply with the wrong length.
> 
> Change the error message to show it is an length check failure with both
> the received and expected values.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
> ---
>  target/i386/kvm/tdx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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